[libvirt] [PATCH] maint: don't permit format strings without %

[Jim Meyering]

Eric Blake wrote:
> Any time we have a string with no % passed through gettext, a
> translator can inject a % to cause a stack overread.  When there
> is nothing to format, it's easier to ask for a string that cannot
> be used as a formatter, by using a trivial "%s" format instead.
>
> In the past, we have used --disable-nls to catch some of the
> offenders, but that doesn't get run very often, and many more
> uses have crept in.  Syntax check to the rescue!
>
> The syntax check can catch uses such as
> virReportError(code,
>                _("split "
>                  "string"));
> by using a sed script to fold context lines into one pattern
> space before checking for a string without %.
>
> This patch is just mechanical insertion of %s; there are probably
> several messages touched by this patch where we would be better
> off giving the user more information than a fixed string.
...
> diff --git a/cfg.mk b/cfg.mk
> index d054e5a..085ef34 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -547,7 +547,7 @@ msg_gen_function += xenapiSessionErrorHandler
>  # msg_gen_function += vshPrint
>  # msg_gen_function += vshError
>
> -func_or := $(shell printf '$(msg_gen_function)'|tr -s '[[:space:]]' '|')
> +func_or := $(shell echo $(msg_gen_function)|tr -s '[[:space:]]' '|')
>  func_re := ($(func_or))
>
>  # Look for diagnostics that aren't marked for translation.
> @@ -567,6 +567,19 @@ sc_libvirt_unmarked_diagnostics:
>  	  { echo '$(ME): found unmarked diagnostic(s)' 1>&2;		\
>  	    exit 1; } || :
>
> +# Look for diagnostics that lack a % in the format string, except that we
> +# allow VIR_ERROR to do this, and ignore functions that take a single
> +# string rather than a format argument.
> +sc_prohibit_diagnostic_without_format:
> +	@{ grep     -nE '\<$(func_re) *\(.*;$$' $$($(VC_LIST_EXCEPT));   \
> +	   grep -A2 -nE '\<$(func_re) *\(.*,$$' $$($(VC_LIST_EXCEPT)); } \
> +	   | sed -rn -e ':l; /[,"]$$/ {N;b l;}'				 \
> +		-e '/xenapiSessionErrorHandler/d'			 \
> +		-e '/\<$(func_re) *\([^"]*"([^%"]|"\n[^"]*")*"[,)]/p'	 \
> +           | grep -vE '(VIR_ERROR|vah_(error|warning))' &&		 \
> +	  { echo '$(ME): found diagnostic without %' 1>&2;		 \
> +	    exit 1; } || :
> +
>  # Like the above, but prohibit a newline at the end of a diagnostic.
>  # This is subject to false positives partly because it naively looks for
>  # `\n"', which may not be the end of the string, and also because it takes

Hi Eric,

Nice patch.
The subject piqued my interest, so even on PTO, I took a quick look.
One issue (I haven't gone through the whole thing)
is that you should not insert the new rule there, because
by separating the preceding and following rules, it would
render invalid the following "Like the above..." comment.

Also, I didn't see right off why you've changed from using
printf to using echo in the definition of func_or.
Without looking at the definition of msg_gen_function, I
suspect that switching to echo will add an unwanted trailing "|", no?
Seeing no ChangeLog entry, I wondered if it was an accident.