Re: (ITS#3839) pedantic and format troubles

[hyc@symas.com]

h.b.furuseth@usit.uio.no wrote:
>  Full_Name: Hallvard B Furuseth Version: HEAD OS: URL: Submission
>  from: (NULL) (129.240.186.42) Submitted by: hallvard
>
>
>  gcc -pedantic -Wformat -Wno-format-extra-args isn't nice to the
>  config stuff:
>
>  ===
>
>  ConfigTable.arg_item is set to (void*)integer or (void*)function when
>  arg_type & (ARG_MAGIC | ARG_OFFSET).  This is not portable - function
>  and void pointers may have different sizes, or pointers may be
>  normalized in some way after being assigned integers, etc.

I feel this concern is unwarranted for a number of reasons:
  1) re int vs pointer - any supposed normalization must be reversible. 
We are not converting integers to pointers or vice versa; that is, we do 
not store an integer and later attempt to use it as a pointer. As such, 
if the value we read from this variable is not the same as the value we 
stored, the compiler is broken.
  2) there is a large body of code that depends on the 
interchangeability of function pointers and object pointers, thanks to 
APIs like dlsym() and friends. 
http://www.opengroup.org/onlinepubs/009695399/functions/dlsym.html
The fact that this stuff still works tells me no problem exists. When 
these things stop working, then there's a legitimate issue to fix.

>  This should either be replaced with 3 fields (where only one is set
>  per table entry), or the current field could be set to a pointer to a
>  variable containg the current value.

Feel free to replace this with a union if you wish. I don't see the need 
but also there's no harm if you make this change.

>  Um.  And I guess it's about time to fix the same problem with
>  ldap_pvt_thread_pool_<setkey/getkey>().  They receive function
>  pointers sometimes.  I'll create some static variables to hold these
>  function pointers, and pass them pointers to those variables instead.

Don't bother to preserve the function pointers. The only point in those 
calls is to pass a unique value to distinguish one key from another. It 
is of no interest whether the function pointer is passed *intact* so 
long as it is passed *consistently* in each invocation, so that the same 
value is propagated. The passed pointers are never dereferenced. If you 
don't like using function pointers here, just pass some other unique 
value instead.

>  ===
>
>  ConfigArgs.lineno is an unsigned long, but a number of routines that
>  can report line numbers (like parse_acl) take an int. So do a lot of
>  printf formats.  I started to fix the formats that are wrong, but
>  maybe it's just as well to use int lineno.

I agree, an int is probably sufficient.

>  A ligher change is
>  to make the char* file/mode args to ldif_open() LDAP_CONST, since
>  they are passed const char*'s from elsewhere.

fine.

>  ===
>
>  Two plain bugs in bconfig.c: Message "%s: (optional) %s overlay
>  \"%s\" configuration failed" gets the char c->argv[1][1] instead of
>  c->argv[1]+1 or something.
>
>  Message "<%s> cannot be deleted" gets no arg to the %s. Looks like it
>  should be deleted, since it's repeated below.

OK.

-- 
  -- Howard Chu
  Chief Architect, Symas Corp.       Director, Highland Sun
  http://www.symas.com               http://highlandsun.com/hyc
  Symas: Premier OpenSource Development and Support