char* dynamic strings - the right way

D

drj0nson

What is the right way of creating a string/char array and assigning to
a char* which is then used in a function call. Thought it would be
quite nice to avoid a memory leakage /core dump.

1 Header with char* x

2 Class includes header

3 In Class member function I want to :
a)conditionally create a string ie populate x
b)pass (populated) x on as a function parameter.

What is the right way of doing this for :
i) assigning string variable
ii) assigning a literal.

i) Since it's a conditional create I guess it is not a good idea to use
malloc or new.

ii) Is it ok to just have x = "some string";
 
J

John Harrison

What is the right way of creating a string/char array and assigning to
a char* which is then used in a function call. Thought it would be
quite nice to avoid a memory leakage /core dump.

The right way, the easy way is to use std::string.
1 Header with char* x

2 Class includes header

3 In Class member function I want to :
a)conditionally create a string ie populate x
b)pass (populated) x on as a function parameter.

What is the right way of doing this for :
i) assigning string variable
ii) assigning a literal.

i) Since it's a conditional create I guess it is not a good idea to use
malloc or new.

ii) Is it ok to just have x = "some string";

I think you need to post the actual code you have written, it quite hard
to work out exactly what you mean, and if you really want to do it this
way then the details matter. But as I said the easy way is to drop char*
and use std::string instead.

std::string x;

x = "some string";
some_function(x);

What could be easier? No memory leaks, no core dumps.

john
 
D

drj0nson

John said:
The right way, the easy way is to use std::string.


I think you need to post the actual code you have written, it quite hard
to work out exactly what you mean, and if you really want to do it this
way then the details matter. But as I said the easy way is to drop char*
and use std::string instead.

std::string x;

x = "some string";
some_function(x);

What could be easier? No memory leaks, no core dumps.

john

Dealing with existing code i.e.starting point is that I have a char*
and wish to either populate this in different ways : may call a func
that returns a char* or may wish to assign a string literal. So.

x = f(); being char* f() {...
x = "some string";

First though was to malloc/new to size of string/char array but it is
conditional as to whethr x gets populated.
 
P

peter koch

(e-mail address removed) skrev:
John said:
What is the right way of creating a string/char array and assigning to
a char* which is then used in a function call. Thought it would be
quite nice to avoid a memory leakage /core dump.

The right way, the easy way is to use std::string.
[snip]
std::string x;

x = "some string";
some_function(x);

What could be easier? No memory leaks, no core dumps.

john

Dealing with existing code i.e.starting point is that I have a char*
and wish to either populate this in different ways : may call a func
that returns a char* or may wish to assign a string literal. So.

x = f(); being char* f() {...
x = "some string";

First though was to malloc/new to size of string/char array but it is
conditional as to whethr x gets populated.
The easy way still is to use std::string. You really can't rely on
functions returning char* unless you really know what is going on
inside. You do not know how much memory (if any) has been allocated for
the array, you don't know if you have to free the pointer and you do
not know how to free it. Just start using std::string and encapsulate
your existing code (or rewrite it, if that is easier).

/Peter
 
J

John Harrison

Dealing with existing code i.e.starting point is that I have a char*
and wish to either populate this in different ways : may call a func
that returns a char* or may wish to assign a string literal. So.

x = f(); being char* f() {...
x = "some string";

First though was to malloc/new to size of string/char array but it is
conditional as to whethr x gets populated.

Whose responsibility is it to free the memory returned from f()?

Still dont see why you can't ise std::string

std::string x;

if (something)
x = f();
else
x = "some string";

john
 
E

Eckhard Lehmann

What is the right way of creating a string/char array and assigning to
a char* which is then used in a function call. Thought it would be

First of all, remember: char* is a _pointer_ to a char.
Not a string, not an array or anything else goofy. Just a pointer.
quite nice to avoid a memory leakage /core dump.

If you want to use char*, memory management is up to you. However, if
you use functions that return char*, you may rely on the fact, that the
memory is properly allocated there.
1 Header with char* x

2 Class includes header

3 In Class member function I want to :
a)conditionally create a string ie populate x
b)pass (populated) x on as a function parameter.

What is the right way of doing this for :
i) assigning string variable
ii) assigning a literal.

i) Since it's a conditional create I guess it is not a good idea to use
malloc or new.

ii) Is it ok to just have x = "some string";

What is the problem here? Hard to figure out what you mean, can you post
any code?


Eckhard
 
D

drj0nson

Eckhard said:
First of all, remember: char* is a _pointer_ to a char.
Not a string, not an array or anything else goofy. Just a pointer.


If you want to use char*, memory management is up to you. However, if
you use functions that return char*, you may rely on the fact, that the
memory is properly allocated there.


What is the problem here? Hard to figure out what you mean, can you post
any code?


Eckhard

Ok here is some code but in other places I would deal with char* /
char[] and not a string.

void f2(string &s)
{
s="qwerty";
}

void f1(myStruct &ms)
{
string s;

f2(s);
// Error 203: # Cannot assign 'char *' with 'const char *'.
//ms.cp = s.c_str();

ms.cp = (char*)s.c_str();
}


int main () {
myStruct mystruct;

f1(mystruct);
cout << mystruct.cp << endl;
return 0;
}

Note:
1) am stuck with using a char* - non-negotiable.
2) That I get Error 203 and need to cast seems to indicate this isn't
the correct way.
3) I did wonder if the s.c_str() would go out of scope leaving f1()
i.e. is this dangerous?
4) I would have thought it would be best to allocate memory to the
char* in main() but at that point the string size is unknown.

Sorry for the delay in responding.
 
R

Ron Natalie

2) That I get Error 203 and need to cast seems to indicate this isn't
the correct way.

Do you know what const means?
3) I did wonder if the s.c_str() would go out of scope leaving f1()
i.e. is this dangerous?

The return of c_str() ceases to be valid when a non-const method
(including the destructor) is run on the object.
4) I would have thought it would be best to allocate memory to the
char* in main() but at that point the string size is unknown.
So how would you do it?
 
D

drj0nson

I do not know the correct way of doing this - that is why I am posting
here!

1. You have a char *
2. You need to go off and populate this with a string or char array but
you do not know the size of the string., so....
3. string seems to be 'the' way of doing this but
a) c_str() returns a const and needs casting using char*
b) although the c_str() casting works and it is possible to print
the string out later - is this a case of something that will work
but not always?
 
J

John Harrison

I do not know the correct way of doing this - that is why I am posting
here!

1. You have a char *
2. You need to go off and populate this with a string or char array but
you do not know the size of the string., so....
3. string seems to be 'the' way of doing this but
a) c_str() returns a const and needs casting using char*
b) although the c_str() casting works and it is possible to print
the string out later - is this a case of something that will work
but not always?

You need to allocate memory

string s = "qwerty";
char* ptr = new char[s.size() + 1];
strcpy(ptr, s.c_str());
....
// sometime later when you are done
delete[] ptr;

It's because of this mess (particularly the need to delete[]) that you
should use strings instead of char, if you possibly can.

john
 
D

drj0nson

John said:
I do not know the correct way of doing this - that is why I am posting
here!

1. You have a char *
2. You need to go off and populate this with a string or char array but
you do not know the size of the string., so....
3. string seems to be 'the' way of doing this but
a) c_str() returns a const and needs casting using char*
b) although the c_str() casting works and it is possible to print
the string out later - is this a case of something that will work
but not always?

You need to allocate memory

string s = "qwerty";
char* ptr = new char[s.size() + 1];
strcpy(ptr, s.c_str());
...
// sometime later when you are done
delete[] ptr;

It's because of this mess (particularly the need to delete[]) that you
should use strings instead of char, if you possibly can.

john

thanks John, I was hoping there was a better way.
Maybe that the string for object string s would remain
while it had a reference to it and remove it once that
reference is removed - but maybe thats java.

So my program above becomes:

void f2(string &s)
{
s="qwerty";
}


void f1(myStruct &ms)
{
string s;

f2(s);
// Error 203: # Cannot assign 'char *' with 'const char *'.
//ms.cp = s.c_str();

ms.cp = new char[s.size() + 1];
strcpy(ptr, s.c_str());

// and not ms.cp = (char*)s.c_str();
}


int main () {
myStruct mystruct;

f1(mystruct);
cout << mystruct.cp << endl;
delete [] ms.cp;

return 0;
}
 
W

werasm

int main () {
myStruct mystruct;

f1(mystruct);
cout << mystruct.cp << endl;
delete [] ms.cp;

return 0;
}

To find out whether this will work, I have to see what myStruct looks
like. Looks bad, though. Show us <myStruct> definition, then I'll
comment.

Regards,

Werner
 
D

drj0nson

werasm said:
int main () {
myStruct mystruct;

f1(mystruct);
cout << mystruct.cp << endl;
delete [] ms.cp;

return 0;
}

To find out whether this will work, I have to see what myStruct looks
like. Looks bad, though. Show us <myStruct> definition, then I'll
comment.

Regards,

Werner

Whoops sorry,

typedef struct {
char *cp;
} myStruct ;
 
W

werasm

Whoops sorry,

typedef struct {
char *cp;
} myStruct ;

I've looked at your example. It does not compile, firstly - where is
ptr declared in f1 (I'll assume you've meant ms.cp)? Also, the strcpy
omits copying the NULL terminator. This would mean streaming the
NON-terminated string to std::cout would envoke undefined behaviour, if
I'm not mistaken. All of this could have been achieved using:

int main()
{
std::cout << qwerty << std::endl;
return 0;
}

I don't see the point. Also, myStruct does not overload a copy
constructor/assignment operator/destructor. In general, the code is
bad. The only thing you are doing right, is that you call operator
delete[] when creating with operator new[] - good.

W
 
D

drj0nson

Thank you for looking at the code and apologies for the compile error.
Here is the compiled version:

typedef struct {
char *cp;
} myStruct ;

void f2(string &s)
{
s="qwerty";

}

void f1(myStruct &ms)
{
string s;

f2(s);

ms.cp = new char[s.size() + 1];
strcpy(ms.cp, s.c_str());
ms.cp[s.size()]='\0';
}

int main () {
myStruct mystruct;

f1(mystruct);
cout << mystruct.cp << endl;
delete [] mystruct.cp;

return 0;
}

Please bear in mind I have created a simplified example to illustrate a
problem. The premise is having to deal with populating existing chars :
a one line cout statement isn't really the answer.
 
W

werasm

Thank you for looking at the code and apologies for the compile error.
Here is the compiled version:

typedef struct {
char *cp;
} myStruct ;

In c++ the typedef is redundant, therefore ...

struct myStruct
{
char* cp;
};
.... would have sufficed.
void f2(string &s)
{
s="qwerty";

}

Even for you example, this (above) is unecessary.
void f1(myStruct &ms)
{
string s;

f2(s);

This could be:
std::string s( "qwerty" );

In general, it is good to qualify the std items explicitly.
ms.cp = new char[s.size() + 1];
Yes, this is how you allocate memory for the array. The size allocated
is correct as you require provision for a NULL terminator. s.size does
return the size of string excluding NULL terminator.
strcpy(ms.cp, s.c_str());

I would have used strncpy (or char_traits::copy) over here (actually I
would use strings :) ).

strncpy( ms.cp, s.c_str(), s.size() )[s.size()] = '\0';

Note strncpy actually returns the destination string, therefore you
could use above syntax to terminate.
int main () {
I would not have made char* part of struct.
myStruct mystruct;

char* myChar;
f1(myChar);
f1 then has signature <char*&>. IMO this still does not convey the
intent of the function, and it does not indicate to the receiver of
char* that he is required to delete it. It is less work and has the
same effect as your example though.

Therefore:

void f1( char*& out )
{
std::string qstr( "qwerty" );
const unsigned outSz( qstr.size()+1 );

out = new char[ outSz];
strncpy( out, qstr.c_str(), outSz-1 )[outSz];
}

int main()
{
char* myChar( 0 );
f1( myChar );
if( myChar )
{
std::cout << myChar << std::endl;
delete [] myChar;
}
return 0;
}

Also note that deleting a NULL ptr is valid, but sending NULL ptr to
std::cout may not be.

Kind regards,

W
 
W

werasm

werasm said:
Thank you for looking at the code and apologies for the compile error.
Here is the compiled version:

typedef struct {
char *cp;
} myStruct ;

In c++ the typedef is redundant, therefore ...

struct myStruct
{
char* cp;
};
... would have sufficed.
void f2(string &s)
{
s="qwerty";

}

Even for you example, this (above) is unecessary.
void f1(myStruct &ms)
{
string s;

f2(s);

This could be:
std::string s( "qwerty" );

In general, it is good to qualify the std items explicitly.
ms.cp = new char[s.size() + 1];
Yes, this is how you allocate memory for the array. The size allocated
is correct as you require provision for a NULL terminator. s.size does
return the size of string excluding NULL terminator.
strcpy(ms.cp, s.c_str());

I would have used strncpy (or char_traits::copy) over here (actually I
would use strings :) ).

strncpy( ms.cp, s.c_str(), s.size() )[s.size()] = '\0';

Note strncpy actually returns the destination string, therefore you
could use above syntax to terminate.

Actually, this is not necessary in this case, as the source is
guaranteed to be terminated, and strncpy reads until it finds a
terminator, then pads the rest therefore the above statement can
become:

strncpy( ms.cp, s.c_str(), s.size()+1 );

This will ensure ms.cp is terminated :).
int main () {
I would not have made char* part of struct.
myStruct mystruct;

char* myChar;
f1(myChar);
f1 then has signature <char*&>. IMO this still does not convey the
intent of the function, and it does not indicate to the receiver of
char* that he is required to delete it. It is less work and has the
same effect as your example though.

Therefore:

void f1( char*& out )
{
std::string qstr( "qwerty" );
const unsigned outSz( qstr.size()+1 );

out = new char[ outSz];
strncpy( out, qstr.c_str(), outSz-1 )[outSz];

Note that above statement should actually be...

strncpy( out, qstr.c_str(), outSz );

.... for reasons previously mentioned.
}

int main()
{
char* myChar( 0 );
f1( myChar );
if( myChar )
{
std::cout << myChar << std::endl;
delete [] myChar;
}
return 0;
}

Also note that deleting a NULL ptr is valid, but sending NULL ptr to
std::cout may not be.

Kind regards,

W
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,769
Messages
2,569,580
Members
45,054
Latest member
TrimKetoBoost

Latest Threads

Top