[OPEN-ILS-DEV] C nits: utils.c (Part 3)

[Scott McKellar]

I've been looking at set_proc_title().  I still don't really understand
what this is used for, nor how it relates to the preceding function
init_proc_title(), but I have a few comments just from staring at the
raw code.

Most of this function is concealed in the VA_LIST_TO_STRING macro, 
which allocates a local buffer and populates it with a formatted 
string.  VA_BUF points to the resulting buffer.  (Incidentally this 
macro includes a call to bzero() just before the second call to 
vsnprintf().  In this context, at least, bzero() does nothing useful.)

At the end we use snprintf() to transfer the contents of VA_BUF into
a final buffer prepared previously by init_proc_title().

I don't know where the original format string comes from, but suppose
that it contains the character sequence " %%s".

When vsnprintf() formats the string, it converts the pair of percent
signs to a single percent sign.  Then when we call snprintf() at the
end, it sees the character sequence " %s", which includes a conversion 
specification telling it to insert a character string.  So it looks 
through its parameter list for a string to insert.

Oops.  We didn't pass one.  Coredump.  If we're lucky.

A way to avoid this scenario is to use strncpy() at the end, instead
of snprintf().  Since strncpy() pads on the right with nul bytes, it
would enable us to eliminate the preceding bzero() (which may not be
necessary in any case; I can't tell).  strncpy() will presumably be
faster as well, because it doesn't have to parse a format string
looking for conversion specifications.

Since I don't know where the original format strings come from, I 
don't know that these funky perversities are even possible.  Maybe
they aren't, in which case we don't have a bug, just a needless
inefficiency.  I bring it up because this kind of bug is easy to
overlook.

Scott McKellar
http://home.swbell.net/mck9/aargh/