Re: [PATCH] virsh: Print error message if argument parsing fails for cmdNodesuspend

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/13/2011 03:24 PM, Peter Krempa wrote:
>> Compilation without NLS will trigger gcc warnings that you are using a
>> printf-style interface without any %.  Write this as vshError(ctl, "%s",
>> _("Invalid target argument")).
>>
> 
> Oh! Now I understand why there's used notation "%s", "some const string"
> throughout virsh.

I wish we could get gcc to warn even when compiling with NLS.  clang
also issues the warnings about a constant format string, regardless of
whether NLS is enabled, but we don't compile under clang as often.

(The 'documented' reason that gcc warns only when you disable NLS is
that with NLS, the string is hidden inside a function call of gettext(),
but without NLS, the _() macro is a no-op.  But the fact that gcc can
see through the gettext() call to warn about %d vs. long mismatch makes
me say that this is a weak cop-out argument from the gcc camp, and that
they should fix their consistency bug).

> A quick grep on virsh's source revealed more places
> where this happens and even some strings printed without the translation
> macro. (Well, I was using that format in my previous patches ...). I'll
> look for them and try to fix all the locations in a separate patch.

Sure, bundle them all in as a single, separate cleanup patch; it won't
be the first time we've done that (see, for example, commit 991be604).
Alas, I don't know of any way to teach 'make syntax-check' to be smarter
about it, so it probably also won't be the last.

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]