simple malloc() problem

[hermit_crab67]

Can someone explain to a C newbie why this doesn't work as I expect it
to work? (expectations clearly outlined in the printf statement in
main routine)

OS: Linux 2.4.26
GCC: 2.95.4

void modify_pointer(char *);

int main(int argc, char *argv[]) {
char *ptr_to_string;
modify_pointer(ptr_to_string);
printf("but why doesn't this say hello? %s\n", ptr_to_string);
}

void modify_pointer(char *the_ptr) {
the_ptr = (char *) malloc(strlen("hello"));
strcpy(the_ptr, "hello");
printf("this says hello: %s\n", the_ptr);
}

--
output:

this says hello: hello
but why doesn't this say hello? p�

---

 

[Vijay Kumar R Zanvar]

Can someone explain to a C newbie why this doesn't work as I expect it
to work? (expectations clearly outlined in the printf statement in
main routine)

The problem isn't with the malloc (except the memory leakage here).
ptr_to_string has been passed by value, so the assigment

the_ptr = (char *) malloc(strlen("hello")); /* don't cast */

is local to the modify_pointer(). Do as follows:

OS: Linux 2.4.26
GCC: 2.95.4

void modify_pointer(char *);

void modify_pointer(char **);

int main(int argc, char *argv[]) {
char *ptr_to_string;
modify_pointer(ptr_to_string);

modify_pointer(&ptr_to_string);

printf("but why doesn't this say hello? %s\n", ptr_to_string);
}

void modify_pointer(char *the_ptr) {
the_ptr = (char *) malloc(strlen("hello"));
strcpy(the_ptr, "hello");
printf("this says hello: %s\n", the_ptr);
}

void modify_pointer(char **the_ptr) {
*the_ptr = malloc(sizeof("hello"));
strcpy(*the_ptr, "hello");
printf("this says hello: %s\n", *the_ptr);
}

This might be of some help:

In C, the only true method of parameter passing is known as "call by value".
The following example demonstrates the last statement:

#include <stdlib.h>

void
raisin ( float x ) { /* process x here */ }

void
grape ( float *xp ) { /* process xp here */ }

int
main ( void )
{
float f;
float *fp = &f;

/* ... */
raisin ( f );
grape ( fp );
return EXIT_SUCCESS;
}

When the declaration of an argument (f and fp) matches with the declaration
of the corresponding parameter (x and xp), then the argument has been passed by
value. And, when an argument is passed by value, any modification the called-
function makes to the parameter does not reflect in the calling-function. So,
any modification to xp does not reflect in fp, however, any modification to *xp
reflects in f.

 

[Emmanuel Delahaye]

Can someone explain to a C newbie why this doesn't work as I expect it
to work? (expectations clearly outlined in the printf statement in
main routine)

OS: Linux 2.4.26
GCC: 2.95.4

void modify_pointer(char *);

Starts badly. If you want to modify an object, you must pass the address of
it. Remember, in C, only the values are passed.

void modify_pointer(char **pp);

Your problem is covered by the FAQ:

http://www.eskimo.com/~scs/C-faq/q4.8.html

int main(int argc, char *argv[]) {
char *ptr_to_string;

This pointer is uninitialized.

modify_pointer(ptr_to_string);

Passing an uninitialized value to a function invokes an undefined behaviour.
Anything can happen. A decent and well-configured compiler should have warned
you about that.

Because you are using gcc (note: gcc is not GCC), I suggest you add this to
your command line:

-W -Wall -O2

and the following in addition if you want to comply to the C standard:

-ansi -pedantic

printf("but why doesn't this say hello? %s\n", ptr_to_string);

}

void modify_pointer(char *the_ptr) {
the_ptr = (char *) malloc(strlen("hello"));

Modifiying the value of a parameter is often the indication of a bad design.

No need for the cast. Be sure that you have included <stdlib.h>. malloc() can
fail. You must test the returned value against NULL.

strlen() wants <string.h>. It returns the length of the string, not it's
size. You're off by one.

char * the_ptr = malloc(strlen("hello") + 1);

or, in this special case involving a string literal:

char * the_ptr = malloc(sizeof "hello");

strcpy(the_ptr, "hello");

But it is recommended to avoid such repetitions.
- Not all compilers are smart enough to merge similar string literals, hence
a waste of space.
- It makes maintenance a hell

I recommend an initialized array of const char:

char const s[] = "hello";
char * the_ptr = malloc(strlen(S) + 1);

if (the_ptr != NULL)
{
strcpy(the_ptr, S);

printf("this says hello: %s\n", the_ptr);
}

Not to mention that the value of 'the_ptr' must now be returned via the new
char ** pp parameter (after having checked that pp is not NULL, of course).

if (pp != NULL)
{
*pp = the_ptr;
}

 

[Case]

Modifiying the value of a parameter is often the indication of a bad design.

I'm interested in your thoughts behind this statement.

Case

 

[Emmanuel Delahaye]

I'm interested in your thoughts behind this statement.

The code given by the original poster is a perfect illustration of this
statement.

Apart for micro-optimisation purpose, there is not good reasons for a
parameter to have its value changed. This is the reason why, at a first
glance, I recommend to define the parameters with const. If it clashes
somewhere, it means that it's time to think harder about the design.

 

[CBFalconer]

I'm interested in your thoughts behind this statement.

This is a religious issue, on which E.D. and I disagree. I treat
parameters as externally initialized local variables (which they
are). If a function is so long that I can't easily spot the need
for the original value, then there is a design problem and the
function needs to be broken up into smaller and simpler functions.

 

[Dan Pop]

Like most such statements made by Emmanuel, it's a matter of pure
religion taken as fact.

The code given by the original poster is a perfect illustration of this
statement.

And the following code is a perfect rebuttal of the same statement:

#include <stdio.h>

int main(int argc, char **argv)
{
while (argc-- > 0) puts(*argv++);
return 0;
}

At the end of the loop, the initial values of argc and argv are
entirely irrelevant, so there is no *good* (i.e. non-religious) reason
for preserving them.

And although this is a trivial example, it is typical for code parsing
its command line arguments. See also the example at page 117 in K&R2.

Dan

 

[Guest]

Like most such statements made by Emmanuel, it's a matter of pure
religion taken as fact.


And the following code is a perfect rebuttal of the same statement:

#include <stdio.h>

int main(int argc, char **argv)
{
while (argc-- > 0) puts(*argv++);
return 0;
}

At the end of the loop, the initial values of argc and argv are
entirely irrelevant, so there is no *good* (i.e. non-religious) reason
for preserving them.

Appreciated.

- Dario

 

[Joona I Palaste]

Like most such statements made by Emmanuel, it's a matter of pure
religion taken as fact.

As it stands, I happen to agree with him. Emphasis on "often the
indication of" bit. There are legitimate causes for modifying the value
of a parameter but usually it's a sign of the programmer not
understanding how C's parameter passing works.

 

[Emmanuel Delahaye]

Like most such statements made by Emmanuel, it's a matter of pure
religion taken as fact.

I was naively thinking that Usenet was not the place for nominative attacks.
Sounds that I was wrong. My statements belong to my coding guidelines that
result from a rather long experience in C-programming. You may gently agree
or not, byt why being so harsh ?

And the following code is a perfect rebuttal of the same statement:

#include <stdio.h>

int main(int argc, char **argv)
{
while (argc-- > 0) puts(*argv++);
return 0;
}

At the end of the loop, the initial values of argc and argv are
entirely irrelevant, so there is no *good* (i.e. non-religious) reason
for preserving them.

What a strange example. What if I now want to access to the arguments ? Or to
check the number of arguments?

Your example illustrates exactly what I try to fight against : the
destruction of the initial value of a parameter.

And although this is a trivial example, it is typical for code parsing
its command line arguments. See also the example at page 117 in K&R2.

I think that I'm not the one that consider the K&R2 a Sacred Book. Who is the
one with a religious attitude ?

 

[Martin Ambuhl]

Can someone explain to a C newbie why this doesn't work as I expect it
to work? (expectations clearly outlined in the printf statement in
main routine)

The real questions are
1) Why are you ignoring the diagnostics from the compiler
2) If you don't see any diagnostics, why do you have them turned off?
3) Why don't you check the FAQ before posting, as civilized human beings do?

A fixed version of your code follows the reproduction of your original code.

OS: Linux 2.4.26
GCC: 2.95.4

If the particular operating system or compiler were relevant to your
question (they aren't), then your question would be a heavy favorite to
be off-topic here and to belong in a Linux or GCC newsgroup or mailing list.

void modify_pointer(char *);

int main(int argc, char *argv[]) {
char *ptr_to_string;
modify_pointer(ptr_to_string);
printf("but why doesn't this say hello? %s\n", ptr_to_string);
}

void modify_pointer(char *the_ptr) {
the_ptr = (char *) malloc(strlen("hello"));
strcpy(the_ptr, "hello");
printf("this says hello: %s\n", the_ptr);
}

#include <stdio.h> /* mha: added, variadic printf _must_
have a prototype in scope */
#include <stdlib.h> /* mha: added */
#include <string.h> /* mha: added */

void modify_pointer(char **); /* mha: added level of indirection */

int main(void)
{
char *ptr_to_string;
modify_pointer(&ptr_to_string); /* mha: added address-of operator */
printf("but why doesn't this say hello? %s\n", ptr_to_string);
free(ptr_to_string); /* mha: added */
return 0; /* mha: added */
}

void modify_pointer(char **the_ptr)
{ /* mha: added level of indirection */
#if 0
the_ptr = (char *) malloc(strlen("hello"));
#endif
/* mha: the above fixed below */
if (!(*the_ptr = malloc(1 + strlen("hello")))) {
fprintf(stderr, "Malloc failed, quitting...\n");
exit(EXIT_FAILURE);
}
strcpy(*the_ptr, "hello"); /* mha: added level of indirection */
printf("this says hello: %s\n", *the_ptr); /* mha: added level of
indirection */
}

 

[Dan Pop]

As it stands, I happen to agree with him. Emphasis on "often the
indication of" bit. There are legitimate causes for modifying the value
of a parameter but usually it's a sign of the programmer not
understanding how C's parameter passing works.

Nonsense! It is only very green newbies that *might* expect changes made
to a function parameter to be visible from the caller. Not even them,
with a good instructor/book.

Once you understand that parameters are locals initialised by the caller,
you have no reason not to change their values, and if you have such a
reason, declare them as const.

Dan

 

[Dan Pop]

I was naively thinking that Usenet was not the place for nominative attacks.

I can see no attack, merely an accurate statement of fact.

Sounds that I was wrong. My statements belong to my coding guidelines that
result from a rather long experience in C-programming. You may gently agree
or not, byt why being so harsh ?

Because, *far too often*, you present your religion as a matter of fact,
which it isn't!

What a strange example. What if I now want to access to the arguments ? Or to
check the number of arguments?

Then, you code it differently. Note, however, that, since this is a
*complete* program, you don't want to do anything else than it is already
being done.

Your example illustrates exactly what I try to fight against : the
destruction of the initial value of a parameter.

Which is NOT any more sacred than the initial value of any of the
function's local variables. But you seem to have a difficulty
understanding that.

^^^^^^^^^^^^^^^^^^^^^^^^^^

I think that I'm not the one that consider the K&R2 a Sacred Book.

Neither am I, especially considering that this particular example has
some problems under standard C ;-) However, the more experienced I get,
the more reasons I have to appreciate this book.

Who is the one with a religious attitude ?

How do you parse "see also"? How do you parse the underlined text above?
Is there any connotation that this is the one and only right way of doing
it? Nope. It is a strong statement that things can be done this way and
there is nothing wrong with doing them this way, but there is no
implication that doing them differently would be actually wrong. Compare
that with your statement which says: "it is wrong to do things this way".

You're saying "it is wrong if, at the end of the function's execution,
the parameters don't have their initial values" while I am NOT saying
"it is wrong if, at the end of the function's execution, the parameters
still have their initial values". So, my position cannot be religious,
*by definition*. That is, assuming that we both have the same idea
about what "religion" means.

The right decision must be made on a case by case basis rather than
being a priori decided. No place here even for a "most often".
This is called "pragmatism" and it *is* my "religion" ;-)

Dan

 

[Mark McIntyre]

This is the reason why, at a first
glance, I recommend to define the parameters with const.

well.... I disagree. Modifying a pointer parameter is pretty common
practice I think.

If it clashes somewhere, it means that it's time to think harder about the design.

And if its not a pointer parameter, then modifying it is no different to
modifying any other local variable.

 

[Mark McIntyre]

I was naively thinking that Usenet was not the place for nominative attacks.
Sounds that I was wrong. My statements belong to my coding guidelines that
result from a rather long experience in C-programming. You may gently agree
or not, byt why being so harsh ?

its just dan's way of interacting with humans. Just ignore it.

 

[Al Bowers]

void modify_pointer(char *);

Starts badly. If you want to modify an object, you must pass the address of
it. Remember, in C, only the values are passed.

void modify_pointer(char **pp);


.....snip...


char const s[] = "hello";
char * the_ptr = malloc(strlen(S) + 1);

if (the_ptr != NULL)
{
strcpy(the_ptr, S);
<...>
}

Not to mention that the value of 'the_ptr' must now be returned via the new
char ** pp parameter (after having checked that pp is not NULL, of course).

if (pp != NULL)
{
*pp = the_ptr;
}

This is rather poor advise. Why test for pp being a NULL value
after having attempted the dynamic allocation? If pp is NULL the malloc
and strcpy statements would be rattlebrain.

I would suggest the op modify the function prototype and definiton
to include a return value that can be tested for success/failure of the
modification and an extra parameter that would indicated the source
of the amendment.

char * modify_pointer(char **pp, const char *newstring);

The definition and useage:

#include <stdlib.h>
#include <string.h>
#include <stdio.h>

char *modify_pointer(char **pp, const char *teststring)
{
char *tmp;

if(pp == NULL || teststring == NULL) return NULL;
if((tmp = realloc(*pp,strlen(teststring)+1)) == NULL)
return NULL;
strcpy(tmp,teststring);
return (*pp = tmp);
}

void myfree(char **s)
{
free(*s);
*s = NULL;
return;
}

int main(void)
{
char *s = NULL; /* Initial NULL value neccessary */

if(modify_pointer(&s,"Hello"))
printf("s points to string: \"%s\"\n",s);
else puts("pointer s is a null pointer");
puts("\nAttempting modify.....\n");
if(modify_pointer(&s,"Goodbye"))
printf("s points to string: \"%s\"\n",s);
else puts("pointer s is a null pointer");
myfree(&s);
return 0;
}

 

[Darrell Grainger]

As it stands, I happen to agree with him. Emphasis on "often the
indication of" bit. There are legitimate causes for modifying the value
of a parameter but usually it's a sign of the programmer not
understanding how C's parameter passing works.

I didn't read the same thing from Emmanuel's posting. A bad DESIGN is not
a bad program. A programmer not understanding how parameters are passed in
C is not a bad design; that is just bad programming.

To say that by DESIGN I should not modify the input parameters to a
function is a matter of choice. If the program does what is intended and
the code is maintainable then it becomes a matter of opinion.

If Emmanuel meant that modifying the input parameters to a function is
usually the result of a programmer misunderstanding how parameters are
passed in C then I would agree. I see more programmers doing this because
they misunderstand parameter passing than because they just like to reuse
variables.

If anyone tells me that you must never modify the input parameters to a
function then I take that as their opinion.

 

[Darrell Grainger]

I was naively thinking that Usenet was not the place for nominative attacks.
Sounds that I was wrong. My statements belong to my coding guidelines that
result from a rather long experience in C-programming. You may gently agree
or not, byt why being so harsh ?

Dan does this to everyone. He has done it to me. I don't take it
personally. I've actually learn to question things other people tell me as
fact. I've come to realize that many things I take as fact are just my
opinion.

If I was working for Dan and he wanted to modify input parameters, so be
it. If I'm working for you and you state, no modifying input parameters,
never will an input parameter be modified.

Why? Because it is such a trivial matter.

What a strange example. What if I now want to access to the arguments ? Or to
check the number of arguments?

Do not modify the input parameters if you know you are going to need them
more than once. If you need to access them twice it would be foolish to
use them once in a way that modified them.

You might counter with, "what if you designed it such that you modified
the input parameters and later (version 2.0 perhaps) realized you needed
to access the parameters twice?" Simple, put a piece of code at the top of
the function that saves the input paramters into backup variables. Later
when you need to access the input parameters a second time, use the backup
variables. Puts you right back to where you would have been if you didn't
modify the input parameters in the first place.

Your example illustrates exactly what I try to fight against : the
destruction of the initial value of a parameter.


I think that I'm not the one that consider the K&R2 a Sacred Book. Who is the
one with a religious attitude ?

I took his reference to K&R2, "here is a trivial example, if you want to
see a more complex example it is on page 117... I'm just too lazy to type
in the example myself."

Bottom line, Dan can be a little harsh at times but you have to look at
what he is saying rather than taking it personally by looking at how he is
saying it.

 

[Chris Torek]

Do not modify the input parameters if you know you are going to need them
more than once. If you need to access them twice it would be foolish to
use them once in a way that modified them.

You might counter with, "what if you designed it such that you modified
the input parameters and later (version 2.0 perhaps) realized you needed
to access the parameters twice?" Simple, put a piece of code at the top of
the function that saves the input paramters into backup variables. ...

Or, as I often do:

void fn(int iparm, char *sparm) { /* first version */
... code that modifies iparm and sparm ...
}

becomes:

void fn(int iparm0, char *sparm0) { /* second version */
int iparm = iparm0;
char *sparm = sparm0;
... code that *still* modifies iparm and sparm ...
}

The original values are now available under the new names iparm0
and sparm0 as needed. (I *might* even make those "const" at this
point, just for emphasis, but probably not. As a rule I try not
to modify x-sub-nought though -- this is sort of a C translation
of the mathematical notion of a sequence, where x-sub-0 is the
initial state and each x-sub-i is derived from the previous state.)

I also (perhaps inconsistently) tend to use this zero-suffix for
incoming pointer values that, for interface reasons, have type
"void *" instead of the desired type:

int xyz_interrupt(void *sc0) {
struct xyz_softc *sc = sc0;
...
return flag_indicating_whether_it_was_this_device_that_interrupted;
}

In general I do not use "const" much at all. I think the two main
reasons I avoid peppering C code with this spice are because I have
been using C since before it existed, and because const does not
really work right in C. (See the earlier posting about "const int
arr[][SIZE]" as a parameter, which really means "const int
(*arr)[SIZE]", which cannot be used to point to the various rows
of a regular "int somearr[SOMEROWS][SIZE]" variable.)

 

[hermit_crab67]

Thanks all for the help.

I think I understand the issues at a conceptual level much better
after reading these posts. Basically if I understand it correctly,
the contents that the pointer points to can be modified by the local
function, and those changes will persist after the function loses
scope, but the *pointer itself* cannot be, and only a copy will be
modified -- and that copy will disappear once the function goes out of
scope. Since malloc() is modifying the pointer itself, those changes
are discarded once the method goes out of scope.

This can be corrected by passing in a pointer to a pointer, for one
more level of indirection, (as posted) or the pointer can be
initialized/malloc'd outside of the local function. (discovered via
trial/error)

Next time I will check the FAQ, and my Makefile now has those warning
flags. Posting etiquette tips duly noted.