Jakub Hrozek wrote:
On 08/11/2009 09:29 PM, Dmitri Pal wrote:
>>> 8) in elapi_log.c, the check for handle->appname == NULL on line 228
>>>>> should probably be moved before the TRACE_FLOW_STRING on line
225 as the
>>>>> trace macro tries to access the memory in handle->appname. Also
maybe
>>>>> it's OK to just return ENOMEM and don't bother with errno.
>>>>>
>>>>>
> As for errno it is a valid way of handling error so I guess no change
> needed.
> I prefer errno, I can't explain why...
> Added ticket to address it in a separate patch since fix should be in
> the macro not in the ELAPI code.
>
https://fedorahosted.org/sssd/ticket/99
Sorry I meant something else, I should have explained better..
The problem is, that when strdup() fails, it sets errno to ENOMEM. But
then you call the tracing macro, which calls printf() internally and
this call resets the errno value. After that, you assign the errno to
error if the pointer is NULL, but by that time, errno has the value from
printf, not strdup().
So instead of:
strdup()
TRACE_FLOW_STRING()
error = errno;
The order should be:
strdup()
error = errno
TRACE_FLOW_STRING()
Apart from this, ack to the patch.
Jakub
Ah! Ok, that makes sense.
I opened a ticket (103) to review the collection, INI and ELAPI code
about the use of errno.
I tried not to make this mistake but it seems like I did and you caught
it - good!
But this grants a better, a more thorough review of this and other places.
If everything else is Ok please push the patch.
I will then submit different ticket related patches on top of it later.
Thanks
Dmitri
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
--
Thank you,
Dmitri Pal
Engineering Manager IPA project,
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/