Re: [libvirt] [PATCH 2/4] Update modified mac address in place in virGetInterface

On 07/22/2009 11:36 AM, Daniel P. Berrange wrote: >On Wed, Jul 22, 2009 at 11:25:38AM -0400, Laine Stump wrote: > >>On 07/22/2009 10:20 AM, Daniel Veillard wrote: >> >>>Agreed, patch applied, I only had to add _() to get the message >>>localized, >>> >>I've idly wondered about that macro, as it causes scores of compile >>warnings, like this: >> >>datatypes.c:291: warning: format not a string literal and no format >>arguments >> >>What's it for? And what's the best way to change things to eliminate the >>warnings? >> > >It is a potential security hole, if the user finds a way to send a string >with an embeded format specifier. Thus if you're not doing any >subsitutions, >and the string isn't constant, then you should always at least do > > char *therealstring = ...from somewhere untrusted... > printf("%s", therealstring) > Yeah, I understand the perils of using a non-literal string as the format to *printf(). I'm wondering what the purpose of _() is, and why it causes the compiler to believe the string isn't a literal. (I received this warning when others don't because I use "-Wformat -Wformat-security" in my CFLAGS, at Jim's suggestion).

> >NB, anyone sending patches should always set > > --enable-compile-warnings=error > >when running 'autogen.sh' and make sure all warnings & errors are fixed >before submitting a patch. > It's actually because I like doing this that I'd like to know the preferred method of eliminating the warnings I mentioned. There are a bunch of them pre-existing in the code that I want to get rid of so I can turn on warnings=error (without turning off these warnings in CFLAGS), and I want to do it the "accepted" way. For example, from domain_conf.c:2137: virDomainReportError(conn, VIR_ERR_XML_ERROR, _("invalid security type")); spits out the warning. We all know that it really *is* literal, but the macro is changing the class so the compile thinks it isn't. It would be simple to just change it to: virDomainReportError(conn, VIR_ERR_XML_ERROR, "%s", _("invalid security type")); (and there are plenty of those too), but that's inefficient, and doesn't do the _() around the "%s" (is that correct or not?).