Re: [Libguestfs] [PATCH] Replace shell_quote function with %Q and %R printf specifiers.
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