> Andreas Ericsson wrote:
>> Andreas Ericsson wrote:
>>> Ethan Galstad wrote:
>>>> Andreas Ericsson wrote:
>>>>> Ahoy.
>>>>>
>>>>> I was digging around trying to fix a bunch of compiler-warnings, and
>>>>> noticed that pretty much every invocation of my_free() resulted in
>>>>> about a million warnings, such as this:
>>>>>
>>>>> ../xdata/xrddefault.c:190: warning: dereferencing type-punned
>>>>> pointer will break strict-aliasing rules
>>>>>
>>>>>
>>>>> Considering that not a single invocation of my_free() evaluates the
>>>>> return code of the function, a macro such as
>>>>>
>>>>> #define my_free(ptr) { if(ptr) { free(ptr); ptr = NULL; } }
>>>>>
>>>>> would do the trick, and also provide a slight performance improvement.
>>>>>
>>>>> Since a patch would be fairly huge, and I've kinda filled my quota
>>>>> for huge patches for today, the following sed-script will take care
>>>>> of the call-sites (requires sed 4.0.9 or later):
>>>>>
>>>>> sed -i 's/my_free[^&]*&\([^)]*\).*/my_free(\1);/' */*.c
>>>>>
>>>> Definitely a good idea. I've committed this to CVS after some
>>>> manual massaging post-sed.
>>>>
>>> Thanks.
>>>
>>>> Don't know why, but I get a SIGABRT if I don't comment out two
>>>> my_free() statements in xdata/xodtemplate.c in the
>>>> xodtemplate_get_inherited_string() function.
>>>>
>>> I'll look into it.
>>>
>>
>> Hmm. I can't reproduce it, as I don't have a decent config with template
>> inheritance using + concat stuff. Have you got anything readily made to
>> share? I'll hack some up otherwise, but I'm not sure I'd get one that
>> could trigger it.
>>
>> I'm fairly certain what's happening though. The caller passes the string
>> by reference, but we're free()'ing it as if it wasn't, so we're basically
>> calling free() on a pointer to the caller's stack frame. Bad thing.
>> Change them from
>>
>> my_free(this_value);
>>
>> to
>>
>> my_free(*this_value);
>>
>> and things might turn out ok. Otherwise, tar up your config and send me
>> what you've got and I'll see what I can do.
>>
>
> Got the config, verified the crash, and here's the patch.
>
[snip]
Actually, the real fix is much bigger. The my_free() function should
always be passed a pointer to a pointer. Example:
char *temp_buf=strdup("Something");
my_free(&temp_buf);
/* temp_buf has been free()'d and is now NULL */
The double pointer reference allows my_free() to set the original
pointer to NULL. If this isn't done, the NULL assignment doesn't work
as expected.
I should have caught this when I originally ran sed. The my_free()
macro def has been changed and 1200+ my_free() calls have been modified
to work correctly again. Fix is in CVS.
Ethan Galstad,
Nagios Developer
---
Email: [email protected]
Website: http://www.nagios.org
This post was automatically imported from historical nagios-devel mailing list archives
Original poster: [email protected]