char * memory leak

[Travis]

Quick question, how is a char * different than any other pointer? I've
never had this really elaborated.

Furthermore, is there a memory leak here (note this is just an example
not an actual implementation I'm using).

// name is an attribute of MyClass
void MyClass::SetName(char *n)
{
char *newName = n;

if ( SetToDefault == true )
{
newName = new char [7];


}

 

[Travis]

My bad. I accidentally clicked "Send" with the keyboard.

// Note: name is an attribute of MyClass is properly destroyed in
~MyClass()
void MyClass::SetName(char *n)
{
char *newName = n;

if ( SetToDefault == true )
{
newName = new char [7];
strcpy(newName, "Travis");
}

name = newName;
}


So is this a leak?

 

[Default User]

Quick question, how is a char * different than any other pointer? I've
never had this really elaborated.

It's not really. It has the same representation and alignment as a
void*.

Furthermore, is there a memory leak here (note this is just an example
not an actual implementation I'm using).

// name is an attribute of MyClass
void MyClass::SetName(char *n)
{
char *newName = n;

if ( SetToDefault == true )
{
newName = new char [7];

You're missing a closing brace.

}

Assuming that SetToDefault can be true at some point, then yes. When
control leaves the function, the memory is still allocated but there's
no longer a pointer to it in existence. The assignment of n to newName
is pointless.




Brian

 

[Default User]

My bad. I accidentally clicked "Send" with the keyboard.
Grrrrrrr.

// Note: name is an attribute of MyClass is properly destroyed in
~MyClass()
void MyClass::SetName(char *n)
{
char *newName = n;

if ( SetToDefault == true )
{
newName = new char [7];
strcpy(newName, "Travis");
}

name = newName;
}


So is this a leak?

No. It's still pointless to assign n to newname. What do you think that
does? What is the point of even passing anything to SetName? It would
make more sense if you were copying n to NewName.

Save yourself time and trouble, make name a std::string.



Brian

 

[Travis]

My bad. I accidentally clicked "Send" with the keyboard.
Grrrrrrr.



// Note: name is an attribute of MyClass is properly destroyed in
~MyClass()
void MyClass::SetName(char *n)
{
char *newName = n;

if ( SetToDefault == true )
{
newName = new char [7];
strcpy(newName, "Travis");
}

name = newName;
}

So is this a leak?

No. It's still pointless to assign n to newname. What do you think that
does? What is the point of even passing anything to SetName? It would
make more sense if you were copying n to NewName.

Save yourself time and trouble, make name a std::string.

Brian

I would love to change it to a std::string but it's a vast, existing
code base and I am not allowed to change too much of it.

So are you saying there's no potential leak here?

 

[Daniel T.]

Quick question, how is a char * different than any other pointer?

It's different in that it points to chars while other pointers point to
things other than chars. The answer sounds flip, but that really is
all there is.

By convention a char* is used to hold an array of characters and the
last char of the array is set to 0 so the end can be detected easily.

Lastly, a string literal (i.e., text in quotes) has the type of an array
of chars with an implicit 'last character' that equals 0.

Furthermore, is there a memory leak here (note this is just an example
not an actual implementation I'm using).

// name is an attribute of MyClass
void MyClass::SetName(char *n)
{
char *newName = n;

if ( SetToDefault == true )
{
newName = new char [7];

If you use "new" and don't later use "delete" on the thing you newed,
then there is a leak.

 

[Default User]

My bad. I accidentally clicked "Send" with the keyboard.
Grrrrrrr.



// Note: name is an attribute of MyClass is properly destroyed in
~MyClass()
void MyClass::SetName(char *n)
{
char *newName = n;

if ( SetToDefault == true )
{
newName = new char [7];
strcpy(newName, "Travis");
}

name = newName;
}

So is this a leak?

No. It's still pointless to assign n to newname. What do you think
that does? What is the point of even passing anything to SetName?
It would make more sense if you were copying n to NewName.

Save yourself time and trouble, make name a std::string.

Brian

I would love to change it to a std::string but it's a vast, existing
code base and I am not allowed to change too much of it.

So are you saying there's no potential leak here?

Not the way it's shown. However, that's assuming that name was not
pointing at any allocated memory prior to the assignment. Without
seeing the bigger picture, it's hard to say.

There are things that don't make sense. I don't even see the purpose of
NewName at all, or n, certainly not the assignment of n to NewName.




Brian

 

[Daniel T.]

My bad. I accidentally clicked "Send" with the keyboard.

// Note: name is an attribute of MyClass is properly destroyed in
~MyClass()

Is it? What is the type of 'name'? When it is "properly destroyed" is
the seven element array deleted? Why didn't you show the code?

void MyClass::SetName(char *n)
{
char *newName = n;

if ( SetToDefault == true )
{
newName = new char [7];
strcpy(newName, "Travis");
}

name = newName;
}

So is this a leak?

My answer stays the same, if you new an array of 7 chars, and you don't
ever delete that array, then the above is a leak. We can't know if that
is the case, because we don't know what the rest of the code looks like.

So I ask you: Do you new an array in the above (i.e., is SetToDefault
ever true?) Do you make any attempt at deleting the array that you newed?

If your answers are "yes", and "no", then there is a leak.
If your answers are "no" and "no", then you have no leak.
If your answers are "yes" and "yes", then you will have to show us the
code that deletes the array and the pointer chain that leads from the
newed point to the deleted point before we can tell if there is a leak.
If your answers are "no" and "yes", then you don't have a leak, but you
may have undefined behavior. We would have to see the rest of the code
to know for sure.

 

[Eric]

My bad. I accidentally clicked "Send" with the keyboard.

// Note: name is an attribute of MyClass is properly destroyed in
~MyClass()

Is it? What is the type of 'name'? When it is "properly destroyed" is
the seven element array deleted? Why didn't you show the code?

void MyClass::SetName(char *n)
{
     char *newName = n;

     if ( SetToDefault == true )
     {
          newName = new char [7];
          strcpy(newName, "Travis");
     }

     name = newName;
}

So is this a leak?

My answer stays the same, if you new an array of 7 chars, and you don't
ever delete that array, then the above is a leak. We can't know if that
is the case, because we don't know what the rest of the code looks like.

So I ask you: Do you new an array in the above (i.e., is SetToDefault
ever true?) Do you make any attempt at deleting the array that you newed?

If your answers are "yes", and "no", then there is a leak.
If your answers are "no" and "no", then you have no leak.
If your answers are "yes" and "yes", then you will have to show us the
code that deletes the array and the pointer chain that leads from the
newed point to the deleted point before we can tell if there is a leak.
If your answers are "no" and "yes", then you don't have a leak, but you
may have undefined behavior. We would have to see the rest of the code
to know for sure.

I think perhaps the code below would be better:

~MyClass()
void MyClass::SetName(char *n)
{
char *newName = n;

if ( SetToDefault == true )
{
if(newName)
{
delete [] newName;
}

newName = new char [7];
strcpy(newName, "Travis");
}

name = newName;

}

 

[Daniel T.]

Travis wrote:
// Note: name is an attribute of MyClass is properly destroyed in
~MyClass()
void MyClass::SetName(char *n)
{
char *newName = n;

if ( SetToDefault == true )
{
newName = new char [7];
strcpy(newName, "Travis");
}

name = newName;
}

So is this a leak?

No. It's still pointless to assign n to newname. What do you think
that does? What is the point of even passing anything to SetName?
It would make more sense if you were copying n to NewName.

Save yourself time and trouble, make name a std::string.

Brian

I would love to change it to a std::string but it's a vast, existing
code base and I am not allowed to change too much of it.

So are you saying there's no potential leak here?

Not the way it's shown.

Brian, careful what you say to someone new to the language, there are
potentially several leaks in the above.

However, that's assuming that name was not pointing at any allocated
memory prior to the assignment.

Brian, careful what you say to someone new to the language, there are
potentially several leaks in the above. What if 'n' points to an array
that was newed? What if it points to a single object that was newed? How
does the destructor know if 'name' holds an array that was newed, or an
array that was not newed (or a single object that was newed?)

Without seeing the bigger picture, it's hard to say.

Try impossible.

 

[Alf P. Steinbach]

* Travis:

Quick question, how is a char * different than any other pointer? I've
never had this really elaborated.

That's not a quick question.

'char' is the smallest addressable unit of memory in C and C++.

First, this means a char* pointer may on some architectures need to be
larger than other data pointers (excluding member data pointers), and in
effect it probably determines the size of void*, which must be large
enough for any data pointer.

Second, it means that char* (or variants signed char* or unsigned char*)
is the pointer type you need to use do byte-level memory addressing,
e.g. for traversing data structures defined at the assembly level or in
file headers.

The above two points are not entirely language-specific. For example,
as I recall in Turbo Pascal you had to use pChar (effectively the same
as C++ char*) in order to do pointer arithmetic, which could not be done
using other pointer types. The C++ properties of char* derive from
general considerations which probably were the same that influenced the
design of Turbo Pascal in this regard.

Apart from that, char* in C++ is a bit more unsafe than other pointer
types because C++ freely converts a literal string to char* instead of
only char const*, for backwards compatibility with C, and because many
old C functions expect char* argument instead of char const*.

Finally, due to the properties of 'char' you can in practice always
safely reinterpret_cast a char* to an (un)signed char* and vice versa,
although this is not directly supported by the standard. You can't do
that in a portable way for e.g. int*, because (disclaimer: this has been
debated infinitely many times without a totally clear and absolutely
convincing conclusion) int allows a trap representation.


Cheers, & hth.,

- Alf

 

[Daniel T.]

I think perhaps the code below would be better:

~MyClass()
void MyClass::SetName(char *n)
{
char *newName = n;

if ( SetToDefault == true )
{

The if below is unnecessary, delete knows how to handle a NULL pointer.

if(newName)
{
delete [] newName;
}

newName = new char [7];
strcpy(newName, "Travis");
}

name = newName;

}

What if the array pointed to by 'n' wasn't newed and SetToDefault equals
true?

 

[Eric]

I think perhaps the code below would be better:

~MyClass()
void MyClass::SetName(char *n)
{
     char *newName = n;

     if ( SetToDefault == true )
     {

The if below is unnecessary, delete knows how to handle a NULL pointer.

          if(newName)
          {
             delete [] newName;
          }

          newName = new char [7];
          strcpy(newName, "Travis");
     }

     name = newName;

}

What if the array pointed to by 'n' wasn't newed and SetToDefault equals
true?

thx,: )

so, it depends,does it?

 

[Daniel T.]

I think perhaps the code below would be better:

~MyClass()
void MyClass::SetName(char *n)
{
? ? ?char *newName = n;

? ? ?if ( SetToDefault == true )
? ? ?{

The if below is unnecessary, delete knows how to handle a NULL pointer.

? ? ? ? ? if(newName)
? ? ? ? ? {
? ? ? ? ? ? ?delete [] newName;
? ? ? ? ? }

? ? ? ? ? newName = new char [7];
? ? ? ? ? strcpy(newName, "Travis");
? ? ?}

? ? ?name = newName;

}

What if the array pointed to by 'n' wasn't newed and SetToDefault equals
true?

thx,: )

so, it depends,does it?

Yes. The OP is asking a question that cannot be answered without seeing
a lot more code that what he presented.

 

[James Kanze]

[...]

Brian, careful what you say to someone new to the language,
there are potentially several leaks in the above.

There are potentially memory leaks any time you allocate memory
dynamically. For the most part, there should only be two real
cases where memory is allocated dynamically: within objects with
a variable size (like strings, vectors, etc.)---in which case,
of course, the object manages the memory, and there should be no
leak, or for objects which have explicit lifetimes (entity
objects, generally not copiable)---in which case, the object
itself manages its lifetime (but you have to worry a lot about
dangling pointers). There are a few other odd cases, of course,
but those two probably cover well over 95% of the cases.

In the case in question, of course, as has already been pointed
out, he should be using std::string. If the place where he's
working will not allow him to replace raw, C-like char*'s with
std::string, it's time to change jobs.

 

[Default User]

* Travis:

That's not a quick question.

'char' is the smallest addressable unit of memory in C and C++.

First, this means a char* pointer may on some architectures need to
be larger than other data pointers (excluding member data pointers),
and in effect it probably determines the size of void*, which must be
large enough for any data pointer.

Not probably. void* and char* are required to have the same
representation (and alignment).




Brian

 

[Daniel T.]

There are potentially memory leaks any time you allocate memory
dynamically.

It is possible to statically determine if there is a leak, as long as
enough of the code is presented... so I can't agree that there are
potentially memory leaks any time you allocate memory.

 

[Tobias]

/*
If you really must use char*...

Hope, I didn't miss some bit here.

g++ -g strings.cc
*/

#include <string.h> //< e.g. for strlen
#include <iostream> //< only for demonstration

class MyClass {
enum {maxLen = 100}; // some reasonable length
static char defaultName[]; // the default name
char* name;
bool SetToDefault;
public:
MyClass();
~MyClass() {
delete [] name;
}
void SetName(const char* n);


void IgnoreUserInput() {
SetToDefault = true;
}

void RespectUserInput() {
SetToDefault = false;
}

void whoAmI() {
if(name) { // Always check wether pointer is valid!
std::cout << "My name is " << name << "\n";
}
else {
std::cout << "Ups.\n";
}
}
};

char MyClass::defaultName[] = "DefaultName";

MyClass::MyClass() {
name = new char [ sizeof(defaultName)+1 ];
if(name) { // Always check whether pointer is valid!
strcpy(name,defaultName);
}
SetToDefault = false;
}


void MyClass::SetName(const char *n) { // we don't change *n therefore
'const'.
// DON'T steal ownership of the external string n!!!
size_t len=sizeof(defaultName);

const char *newName; // = n;
if ( SetToDefault || n==NULL /* Handle also invalid input n. */) {
newName = defaultName;
}
else {
len = strlen(n);
if( len <= maxLen ) { //< sanity test of input string length
newName = n;
} else { // You should moan about the error here!
len = sizeof(defaultName);
newName = defaultName;
}
}

delete [] name; // avoid memory leak (name==NULL does not hurt
here)
name = new char[ len + 1 ];
if(name) //< always check
strcpy(name, newName);
// ALWAYS copy to name. Stealing the pointer n from the outside
// causes memory leaks!
}


int main() {
MyClass m;

m.whoAmI();

char someName[] = "SomeName.";

m.SetName(someName);

m.whoAmI();
}

 

[Alf P. Steinbach]

* Default User:

Not probably.

Well I fail to find any hard information on what determines what.

If you have, then that would be nice. Do you?

void* and char* are required to have the same
representation (and alignment).

Yes (that's by §3.9.2/4, which would have been nice if you referred to).


Cheers,

- Alf

 

[Default User]

* Default User:

Well I fail to find any hard information on what determines what.

If you have, then that would be nice. Do you?

The reasoning was that which followed in my next paragraph of my
previous message. The requirement is that void* have the representation
of char*, so it is char* that is the determining factor. Do you feel
that is not the case?

Yes (that's by ?3.9.2/4, which would have been nice if you referred
to).

That's such a basic feature of C++ (and C before it) that I didn't
think it necessary. Sorry.




Brian