two new leaks [Re: 10 more change sets for review

Jim Meyering jim at meyering.net
Wed Oct 20 20:28:28 UTC 2010


Jeff Darcy wrote:

> On 10/20/2010 10:40 AM, Jim Meyering wrote:
>>> I'm inclined to NAK this one.  In general, code should call
>>> malloc/strdup/etc., check the result, and take situationally
>>> appropriate actions on failure.  Calling xstrdup etc. was originally a
>>> crutch for generated code, and its use in code we can actually fix
>>> should be discouraged.  What are the consequences of returning NULL
>>> here?  How hard is that to handle in some way better than exit?
>>
>> Returning NULL is not an option at present -- it would result in
>> a NULL dereference, since the return value can end up being one
>> of the inputs to this function.
>
> Could we return a T_INVALID instead?  That might be easier to handle
> properly in subsequent parsing or evaluation.

Currently it must return a free'able value_t* pointer.
If we've just failed to strdup, a malloc (to allocate
the required buffer) is very likely to fail.

At first I thought of reusing the "left" or "right"
argument, setting it's type to T_INVALID and returning that,
but either of those (or both) may be NULL, since the likes
of make_string and make_number can return NULL.

If we first ensure that no value_t*-returning
function returns NULL, *then* we can do something like the above.

[an hour or two later...]
I think it's done.  A little more testing and I'll post the patch.
That means tomorrow.

While testing the above, I found two new leaks, both provoked by
the replication test:

==19798== 581 (560 direct, 21 indirect) bytes in 1 blocks are definitely lost in loss record 287 of 294
==19798==    at 0x4A04481: calloc (vg_replace_malloc.c:418)
==19798==    by 0x428A6C: access_handler (rest.c:1799)
==19798==    by 0x4E3BB38: ??? (in /usr/lib64/libmicrohttpd.so.5.2.1)
==19798==    by 0x4E3D087: ??? (in /usr/lib64/libmicrohttpd.so.5.2.1)
==19798==    by 0x4E3F9F9: ??? (in /usr/lib64/libmicrohttpd.so.5.2.1)
==19798==    by 0x31BB407760: start_thread (pthread_create.c:301)
==19798==    by 0x31BA8E151C: clone (clone.S:115)

==19694== 5 bytes in 1 blocks are definitely lost in loss record 14 of 366
==19694==    at 0x4A0515D: malloc (vg_replace_malloc.c:195)
==19694==    by 0x42AA5F: proxy_put_attr (rest.c:622)
==19694==    by 0x4E3D473: ??? (in /usr/lib64/libmicrohttpd.so.5.2.1)
==19694==    by 0x4E3F9F9: ??? (in /usr/lib64/libmicrohttpd.so.5.2.1)
==19694==    by 0x31BB407760: start_thread (pthread_create.c:301)
==19694==    by 0x31BA8E151C: clone (clone.S:115)


More information about the iwhd-devel mailing list