[SSSD] [Freeipa-devel] [PATCHES] one for INI another for ELAPI

Dmitri Pal dpal at redhat.com
Wed Aug 12 15:41:20 UTC 2009


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 at 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/




More information about the sssd-devel mailing list