Returning automatic pointers always bad?

[C. J. Clegg]

Consider the following:

char *foo( void )
{
char *bar = (char *)NULL;

bar = malloc( 40 );

return bar;
}

int main( void )
{
char *foobar = (char *)NULL;

foobar = foo();

doSomethingWith( foobar );

free( foobar ); // safe even if foobar is NULL
foobar = (char *)NULL;

return 0;
}

Now, foo( ) is returning an automatic pointer, which as we all know
goes out of scope as soon as foo( ) returns. In this case, that
should be OK, right?... because that pointer is assigned to another
pointer (foobar in main()) which continues to point to valid heap
memory even if bar goes out of scope.

Right?

 

[Richard]

C. J. Clegg said:


The pointer identifier goes out of scope, but the pointer value (if
not NULL) is still valid (because it points to dynamically
allocated memory that you haven't freed yet), and it is this value
that is returned.


Pretty much, yes.

100% yes. In terms of the correctness of returning the value stored in
"bar".

 

[Ben Bacarisse]

Consider the following:

char *foo( void )
{
char *bar = (char *)NULL;

bar = malloc( 40 );

return bar;
}

int main( void )
{
char *foobar = (char *)NULL;

foobar = foo();

doSomethingWith( foobar );

free( foobar ); // safe even if foobar is NULL
foobar = (char *)NULL;

return 0;
}

Now, foo( ) is returning an automatic pointer, which as we all know
goes out of scope as soon as foo( ) returns. In this case, that
should be OK, right?...

Yes. There is a lot of noise in the example, but there is nothing
wrong with it.

because that pointer is assigned to another
pointer (foobar in main()) which continues to point to valid heap
memory even if bar goes out of scope.

Right?

Yes. I wonder what is bothering you. Are you confusing an automatic
pointer variable with a pointer to an automatic variable? Returning
the first is fine, but returning the second is almost always wrong.

 

[Richard]

The code is OK but your reasoning is wrong.
foo returns the same value returned by malloc.
It makes no difference whether or not an automatic variable
once held this value before foo returns.

His reasoning was correct enough if you dig into what he said : "the
pointer is assigned to another pointer which then continues to point to
valid memory". Implicit is "pointer value" when mentioning assigning a
"pointer" in this kind of chat.

 

[Keith Thompson]

Consider the following:

char *foo( void )
{
char *bar = (char *)NULL;

bar = malloc( 40 );

return bar;
}

int main( void )
{
char *foobar = (char *)NULL;

foobar = foo();

doSomethingWith( foobar );

free( foobar ); // safe even if foobar is NULL
foobar = (char *)NULL;

return 0;
}

Now, foo( ) is returning an automatic pointer, which as we all know
goes out of scope as soon as foo( ) returns. In this case, that
should be OK, right?... because that pointer is assigned to another
pointer (foobar in main()) which continues to point to valid heap
memory even if bar goes out of scope.

I think part of the source of your confusion is your use of the term
"pointer" without qualification. A pointer object and a pointer value
are two very different things. In your function foo, bar is a pointer
object; that object ceases to exist when foo finishes. But the
*value* stored in bar is the value that was returned by malloc(40);
that value is perfectly valid even after leaving foo. The return
statement returns (a copy of) the value of bar, not the object bar.

Here's an illustration of the same thing, but using an integer object
rather than a pointer object:

int func(void)
{
int obj = 42;
return obj;
}

The value returned to a caller of func() is 42. The fact that that
value was briefly stored in a local object called "obj" is of no
concern; the value 42 was copied from obj before obj ceased to exist.

Incidentally, the casts in your code, like most casts, are
unnecessary. For that matter, the initializations are unnecessary,
since in both cases you immediately assign a value to the variables.
For example, here's your function foo:

char *foo( void )
{
char *bar = (char *)NULL;

bar = malloc( 40 );

return bar;
}

and here's a series of (in my opinion) improved versions:

char *foo( void )
{
char *bar = NULL;
bar = malloc( 40 );
return bar;
}

char *foo( void )
{
char *bar = malloc( 40 );
return bar;
}

char *foo( void )
{
return malloc( 40 );
}

 

[Richard]

No.
OP is expressing concerns about the value of (bar) being returned
because (bar) has automatic storage.
The assignment to (foobar)
has nothing to do with (bar) being an automatic variable.

OP is mixed up about the difference between
returning the value of an automatic pointer,
which is an unremarkable thing for a function to do
and
returning a pointer to an automatic vaiable,
which is a wrong thing for a function to do.

Yet at no point was a pointer TO an automatic variable mentioned.

You could be right. I personally did not read it that way. But then I
tend to see around corners at times too :-;

And the reason I think this is clear: Its where he says the following:

,----
| >>>> assigned to another
| >>>> pointer (foobar in main()) which continues to point to valid heap
| >>>> memory even if bar goes out of scope.
`----

The "continues to point to" is the pivot for me.

But yes, I take your point as to how you read and can well accept it.

 

[Guest]

Consider the following:

char *foo( void )
{
    char *bar = (char *)NULL;

the cast is unnecessary
char *bar = NULL;

    bar = malloc( 40 );

since bar is immediatly reassigned you don't need to initialise it.
The two lines could be combined. You should check malloc() succeeded.

    return bar;
}

char *foo (void)
{
char *bar = malloc (40);
if (bar == NULL)
abort();
return bar;
}

int main( void )
{
    char *foobar = (char *)NULL;

    foobar = foo();

    doSomethingWith( foobar );

    free( foobar );    // safe even if foobar is NULL
    foobar = (char *)NULL;

    return 0;
}

similar rewrite here

int main (void)
{
char *foobar = foo();
doSomethingWith (foobar);
free (foobar);
foobar = NULL; /* not everyone thinks this is a good idea... */
return 0;
}

Now, foo( ) is returning an automatic pointer,

no. foobar() returns a pointer to a malloc()ed block of memory.

which as we all know
goes out of scope as soon as foo( ) returns.  In this case, that
should be OK, right?...

it is ok

 

[Guest]

(e-mail address removed) said:

Horrors! What right has the foo() function to abort the program? To
the OP: if you must do this, add a comment to the user
documentation for foo, saying something like: "if the function
can't allocate sufficient memory, it aborts the program, making it
completely useless for general programming purposes".

a full blown error handling strategy for a program of this size
seems over the top. And identifier names like foo and bar seem
out of place in a real application. Though I have seen a utility
that used the names of football players for all the identifiers...

Perhaps I should have written

if (bar == NULL)
error ("malloc failure");

 

[Richard]

a full blown error handling strategy for a program of this size
seems over the top. And identifier names like foo and bar seem
out of place in a real application. Though I have seen a utility
that used the names of football players for all the identifiers...

Perhaps I should have written

if (bar == NULL)
error ("malloc failure");

This has been discussed to death.

Total overkill and unlikely to even work in a real malloc failure for
such small memory requirements.

 

[Kaz Kylheku]

Consider the following:

char *foo( void )
{
char *bar = (char *)NULL;

bar = malloc( 40 );

return bar;
}
[snip]

Now, foo( ) is returning an automatic pointer, which as we all know
goes out of scope as soon as foo( ) returns. In this case, that
should be OK, right?

This is just like:

int foo(void)
{
int x = 3;
return x;
}

The object x (the value container) is destroyed, but the value
endures. (In fact you cannot destroy the value 3 itself; you can only
overwrite, with another value, containers which have the value 3).

Pointers are slightly different in that their value /can/ be destroyed. This
happens when the object they point to reaches the end of its lifetime. When
that happens to a pointer value, then it (all copies of it, everywhere in the
program) become an ``indeterminate'' value.

But a malloced object doesn't reach the end of its lifetime until it is
freed (or reallocated). The termination of the function foo is unrelated
to the lifetime of the object returned by malloc(40). That pointer
does not become indeterminate when the function returns.

 

[C. J. Clegg]

OP is mixed up about the difference between
returning the value of an automatic pointer,
which is an unremarkable thing for a function to do
and
returning a pointer to an automatic vaiable,
which is a wrong thing for a function to do.

Not any more. You folks cleared it up. Thanks!