Re: [Outreachy] Return value before or after free()?

[Jonathan Nieder <jrnieder@gmail.com>]

Hi,

Jeff King wrote:
> On Mon, Jan 06, 2020 at 10:15:53PM +0100, Miriam R. wrote:

>> in run-command.c file `exists_in_PATH()` function does this:
>>
>> static int exists_in_PATH(const char *file)
>> {
>>	char *r = locate_in_PATH(file);
>>	free(r);
>>	return r != NULL;
>> }
>>
>> I wonder if it is correct to do return r != NULL; after free(r);
>
> It is technically undefined behavior according to the C standard, but I
> think it would be hard to find an implementation where it was not
> perfectly fine in practice.
>
> Ref: http://c-faq.com/malloc/ptrafterfree.html
>
> I'd probably leave it alone unless it is causing a problem (e.g., a
> static analyzer complaining).

Today I learned.

Miriam, do you have more context?  Did you notice this while reading or
did a tool bring it to your attention?

(Because I was curious, here's what I chased down in C99:

7.20.3.2 "The free function" says: "The free function causes the space
pointed to by ptr to be deallocated, that is, made available for
further allocation."

6.2.4 "Storage durations of objects" says: "The value of a pointer
becomes indeterminate when the object it points to reaches the end of
its lifetime."

6.2.5 "Types" says: "A pointer type describes an object whose value
provides a reference to an entity of the referenced type."

6.5.9 "Equality operators": "Two pointers compare equal if and only if
both are null pointers, both are pointers to the same object
(including a pointer to an object and a subobject at its beginning) or
function, both are pointers to one past the last element of the same
array object, or one is a pointer to one past the end of one array
object and the other is a pointer to the start of a different array
object that happens to immediately follow the first array object in
the address space."

J.2 "Undefined behavior": "The behavior is undefined in the following
circumstances: [...] The value of an object with automatic storage
duration is used while it is indeterminate (6.2.4, 6.7.8, 6.8)"

The reference to automatic storage duration there is interesting.  Of
course `r` here does have automatic storage duration, but the
distinction from

	char **r = xmalloc(sizeof(*r));
	*r = locate_in_PATH(file);
	free(*r);
	/* leak r */
	return *r != NULL;

is peculiar.  It looks like exists_in_PATH is indeed producing
undefined behavior, but the intention of the standard was probably to
make the behavior implementation defined.)

Thanks,
Jonathan