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

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



On 07/24/2012 01:45 PM, Eric Blake wrote:

> The quoted printf version ends up converting the literal leading space
> into '|', giving a regex (|VIR_ERROR|...) for $(func_re) which matches
> _everything_, when used with no further anchors.  Thankfully, we were
> always using $(func_re) with a preceding anchor of \<, and the current
> glibc implementation of \< followed by an empty regex doesn't match
> (although I don't know if that was intentional).
> 
> I switched over to an unquoted printf, so as to let the shell do IFS
> splitting and thus eat the leading space.
> 

>> 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?

Oh, you're right; I swapped a leading empty regex for a trailing empty
regex.  '[[:space:]]' was too broad; checking for just space fixes it
(since $(shell) eats trailing newlines, and tab was already converted to
space by IFS splitting).

>> Seeing no ChangeLog entry, I wondered if it was an accident.
> 
> Intentional, but probably worth an independent commit since I had to
> justify it above.

Now pushed, as an independent commit:

From fc32438fd4b71af6e643d0771a19b4a5c3679ed5 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake redhat com>
Date: Thu, 26 Jul 2012 08:29:15 -0600
Subject: [PATCH] maint: avoid empty regex in syntax checker

We were defining 'func_or' as '|VIR_ERROR|...', which when put
inside 'func_re' resulted in a regex that matches everything in
isolation.  Thankfully, we always used func_re with a leading
anchor \<, and since the empty regex does not start a word, we
happened to get the result we wanted; but it's better to define
func_or without a leading space converted into a leading empty
alternation.

* cfg.mk (func_or): Strip leading space.
---
 cfg.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cfg.mk b/cfg.mk
index 0bf909d..882b2be 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 ' ' '|')
 func_re := ($(func_or))

 # Look for diagnostics that aren't marked for translation.
-- 
1.7.11.2



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

Attachment: signature.asc
Description: OpenPGP digital signature


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