[Libguestfs] [PATCH] Replace shell_quote function with %Q and %R printf specifiers.

[Matthew Booth]

On 27/07/09 22:42, Richard W.M. Jones wrote:
> At the moment the daemon code contains an incredibly hairy function
> called shell_quote for safely quoting strings passed to the shell.
>
> The patch replaces that with a glibc custom printf format (actually
> two, but very closely related), %Q and %R.
>
> %Q is like %s but it safely shell quotes the string.
>
> %R is like %Q but it prefixes the path with /sysroot.
>

Nice.

There's a danger here, though, of increasing the barrier to 
understandability. I can imagine looking at a printf containing %Q or %R 
and wasting a huge amount of time on google fruitlessly trying to work 
out what on earth it's on about. With that in mind I'd definitely add 
something to HACKING. If it weren't for the nowarn wrapper happening to 
provide a weirdness hook, I might also put add a comment above every 
printf which uses it. I would, however, add a much bigger comment to 
asprintf_nowarn, pointing to the implementations.

I also don't think %R is what this feature is about. It seems too 
trivial for a fairly off-the-wall trick. You can just as easily write:

#define SYSROOT "/sysroot"

snprintf (cmd, sizeof cmd, "cat " SYSROOT "%s", path);

It's only slightly longer, and a whole lot more readable to the vast 
majority of C programmers.

Lastly, avoid using inline. The compiler will inline a function if it 
makes sense.

Matt
-- 
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

M:       +44 (0)7977 267231
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490