[Xastir-dev] Proposal to clean up some warnings

[Tom Russo]

On Wed, Jun 09, 2010 at 10:42:52AM -0700, we recorded a bogon-computron collision of the <curt.we7u at gmail.com> flavor, containing:
> On Wed, 9 Jun 2010, Jerry Dunmire wrote:
> 
> > While working on the OSM patch I noticed that the compile produces a
> > lot of warnings and I would like to try and reduce the number a bit.
> >
> > The first one I would like to tackle is:
> >   warning: format not a string literal and no format arguments
> 
> > Does anyone have a problem with this change?
> 
> Yea, me.  I spent a lot of time adding all the xastir_snprintf()
> stuff a while back to make Xastir much more bulletproof.  There are
> hundreds of those calls that were changed at the time.
> 
> I'm not seeing _any_ compiler warnings such as you describe when I
> do a "make clean;make".  

[...]

> I suspect what you're seeing are dependent on your particular
> compiler and it's default flags, along with the CFLAGS that Xastir
> uses.  In my case I'm compiling on OpenSuSE-11.1 or 11.2, 64-bit
> x86.  I have a couple of machines still that run 32-bit, similar OS.


This is a warning produced by newer GCC versions.  I am seeing it on newer
Ubuntu VMs with GCC 4.4.  I believe the intent is to guard against the 
possibility that the string variable might contain formatting characters
that would produce a run time error, e.g. the effect would be similar to 
the obvious mistake:
   snprintf(dest,length,"This %s is an error\n");

The right way to fix it is to change things like this:

  xastir_snprintf(fileimg, sizeof(fileimg), tigertmp);
to
  xastir_snprintf(fileimg, sizeof(fileimg), "%s", tigertmp);

This clears the warning and makes sure that snprintf is really just copying
the string from source to destination.  One has to be careful, though, because
in some cases where we're pulling language codes, the string associated with
the language code has formatting characters in it and is intended to be used
as a format, so its use in xastir_snprintf is not as a copy operation.

I'm not sure why xastir_snprintf is preferrable to strncpy for such a
string copy, but doing it with a %s will definitely clear the warning from
later GCC, and will preserve the meaning of the code so long as only the
   xastir_snprintf(destination,length,string_variable)

instances are the ones that are replaced with
   xastir_snprintf(destination,length,"%s",string_variable)

-- 
Tom Russo    KM5VY   SAR502   DM64ux          http://www.swcp.com/~russo/
Tijeras, NM  QRPL#1592 K2#398  SOC#236        http://kevan.org/brain.cgi?DDTNM
 "It is better to live on your feet than to die with your knees."
  -- Mil Millington on running, in Instructions for Living Someone Else's Life