On (22/01/15 09:15), Roland Mainz wrote:
----- Original Message -----
> From: "Lukas Slebodnik" <lslebodn(a)redhat.com>
> To: "Development of the System Security Services Daemon"
<sssd-devel(a)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(a)redhat.com>
> >> To: "Development of the System Security Services Daemon"
> >> <sssd-devel(a)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).
any integral type shorter than int is promoted to int when passed down to
printf()s variadic arguments. So we can use %d for bool
cat >pok.c <<EOF
#include <stdio.h>
#include <stdint.h>
int main(void) {
int8_t a = 1;
int16_t b = 1;
int32_t c = 1;
printf("a:%d b:%d c:%d\n", a, b, c);
return 0;
}
EOF
sh$ gcc -Wall -Wextra -pedantic pok.c
sh$ echo $?
0
sh$ clang -Wall -Wextra -pedantic pok.c
sh$ echo $?
0
LS