[OpenAFS-devel] Re: [OpenAFS] sprintf -> snprintf...

[R. Lindsay Todd]

Russ Allbery wrote:

>R Lindsay Todd <toddr@rpi.edu> writes:
>
>>How is %p non-portable?
>>
>
>It's not possible to implement %p portably in a pure C snprintf
>implementation, since correctly implementing %p may require knowledge of
>the pointer format of the host platform.  (Think of segmented addressing,
>for example.)  This may or may not be worth worrying about.  I think that
>every platform that OpenAFS currently runs on uses ordinary flat pointers
>that can be meaningfully cast to an intmax_t and then printed with %x.
>
>Basically, the snprintf implementation would have to print pointers with
>%x or the moral equivalent, and therefore our local %p implementation
>would be precisely as non-portable as the existing %x code (which is an
>argument that OpenAFS at present doesn't care).
>
Well, if the platform has a specific way to represent pointers, it is 
true we can't do that without some platform specific knowledge.  But 
having just looked into this a bit more, I see that %p is supposed to be 
a void* pointer.  We know how long these are: sizeof(void*).  We can 
print them out a character at a time, in hex.  This should be portable, 
shouldn't it?  But it the pointers might not print out in the "native" 
format.  If we were willing to use the native sprintf (after testing 
that it actually supports %p), we could even handle this.

What is currently found in OpenAFS is instances of:  sprintf(buf, "%x", 
ptr).  There isn't even a cast, just a presumption that stdarg treats 
ptrs as ints.  These are caught by splint, so they could be fixed either 
by casting to intmax_t, or by implementing %p.

The fact that this "works" is either evidence that it really doesn't 
matter, or that very few people run the volserver in very verbose modes 
and study the logging output.

>>Right now, I don't know of any use of the return code of afs_snprintf.
>>    
>>
>
>Once you start using snprintf widely and properly, particularly if you
>tackle trying to eliminate as many static buffers as you can in userspace
>code, I predict that you will start caring a lot.
>
Yes, this would be useful.  I think glibc has a variant that 
automatically allocates the return buffer; to implement that, the proper 
snprintf return value would be useful, and such a function would go a 
long way to getting rid of static buffers.  Then we would have to make 
sure we haven't introduced memory leaks...

As for your comments on positional parameters:  I think we agree that 
they aren't really needed unless someone wants to start 
internationalizing the code.  There is a lot of stuff with logging that 
needs to be clean up before that happens.  Hopefully going with a 
standard *snprintf will help in that effort.

-- 
R. Lindsay Todd                      email: toddr@rpi.edu
Senior Systems Programmer            phone: 518-276-2605
Rensselaer Polytechnic Institute     fax:   518-276-2809
Troy, NY 12180-3590                  WWW:   http://www.rpi.edu/~toddr