[SSSD] [PATCH] TOOLS: add missing '\n' in debug messages

Roland Mainz rmainz at redhat.com
Thu Jan 22 14:15:29 UTC 2015



----- Original Message -----
> From: "Lukas Slebodnik" <lslebodn at redhat.com>
> To: "Development of the System Security Services Daemon" <sssd-devel at lists.fedorahosted.org>
> Sent: Thursday, January 22, 2015 2:58:26 PM
> Subject: Re: [SSSD] [PATCH] TOOLS: add missing '\n' in debug messages
> On (22/01/15 08:52), Roland Mainz wrote:
> >----- Original Message -----
> >> From: "Lukas Slebodnik" <lslebodn at redhat.com>
> >> To: "Development of the System Security Services Daemon"
> >> <sssd-devel at lists.fedorahosted.org>
> >> Sent: Thursday, January 22, 2015 2:31:13 PM
> >> Subject: Re: [SSSD] [PATCH] TOOLS: add missing '\n' in debug messages
> >> 
> >> On (22/01/15 14:05), Pavel Reichl wrote:
> >> >
> >> >On 01/22/2015 01:27 PM, Pavel Reichl wrote:
> >> >>Hello, please see this trivial patch. Thanks!
> >> >>
> >> >I found some more messages requiring fixing.
> >> 
> >> I found even more
> >> 
> >> --- a/src/providers/ldap/sdap_async.c
> >> +++ b/src/providers/ldap/sdap_async.c
> >> @@ -85,9 +85,9 @@ static void sdap_handle_release(struct sdap_handle *sh)
> >>  
> >>      DEBUG(SSSDBG_TRACE_INTERNAL,
> >>            "Trace: sh[%p], connected[%d], ops[%p], ldap[%p], "
> >> -              "destructor_lock[%d], release_memory[%d]\n",
> >> -              sh, (int)sh->connected, sh->ops, sh->ldap,
> >> -              (int)sh->destructor_lock, (int)sh->release_memory);
> >> +          "destructor_lock[%d], release_memory[%d]\n",
> >> +          sh, sh->connected, sh->ops, sh->ldap,
> >> +          sh->destructor_lock, sh->release_memory);
> >> 
> >> 
> >> It would not be so easy.
> >
> >1. Why did you remove the casts to |(int)| ? IMO the casts are wise in such
> >cases because otherwise you always have to make sure the datatype used in
> >the format string matches the datatype in the |struct| ... and experience
> >tells me that people might change the datatype in the |struct| without
> >crawling over all the code and check the format strings. Please keep the
> >casts.
> 
> I'm sorry but you are missng context.
> 
> variable "sh" is pointer to the "struct sdap_handle"
> which is defined as:
> 
> >struct sdap_handle {
> >    #snip
> >
> >    /* during release we need to lock access to the handler
> >     * from the destructor to avoid recursion */
> >    bool destructor_lock;
> >    /* mark when it is safe to finally release the handler memory */
> >    bool release_memory;
> >};

OK... but that even proves my point since ISO C99 |bool| is not necessarily an |int|. In old C (K&R/ANSI C89/etc.) it was traditionally assumed that |int| is the fastest integer datatype and can be used for boolean values, too. That was long before large caches with complex flush behaviour became common. Today's CPUs work different and usually only flush dirty cache lines as whole chunks to memory and therefore the assumption that |int| is useful as boolean became void. SC22/WG14 therefore decided to recommend to define |_Bool| (typedef'ed to |bool| in <stdbool.h> to the smallest datatype which can hold a boolean value and can have a pointer - this means |char| on so far all platforms I've seen... but: There are no implicit default varargs casting rules defined in C99/C1X for |bool| so a cast to |int| must be done explicitly).

----

Bye,
Roland

-- 
  __ .  . __
 (o.\ \/ /.o) rmainz at redhat.com
  \__\/\/__/  IPA/Kerberos5 team
  /O /==\ O\  
 (;O/ \/ \O;)
 



More information about the sssd-devel mailing list