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

Re: [libvirt] [PATCH v2 01/22] virt-aa-helper: Trick invalid syntax-check



On 11/13/2014 07:37 AM, Martin Kletzander wrote:
> Rule sc_prohibit_newline_at_end_of_diagnostic for syntax-check does
> check for passing strings ending with '\n' to known functions.  But when
> setting IFS, '\n' needs to be part of that (and is by default).  The
> order of individual characters in IFS doesn't matter, so transposing
> last two of them fixes it for the future.
> 
> Signed-off-by: Martin Kletzander <mkletzan redhat com>
> ---
>  src/security/virt-aa-helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index 1f299a0..e102c3c 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -1257,7 +1257,7 @@ main(int argc, char **argv)
>          vah_error(ctl, 1, _("could not set PATH"));
>      }
> 
> -    if (setenv("IFS", " \t\n", 1) != 0) {
> +    if (setenv("IFS", " \n\t", 1) != 0) {

NACK.  IFS is one of those things that even though on paper the POSIX
standard says order doesn't matter for anything besides the first
character, in practice, changing it to anything other than
space-tab-newline is liable to break things.

Rather, reading cfg.mk, I see:

# 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
# two lines of context (the -A2) after the line with the function name.

It looks like the reason you wrote this patch is because once the
_previous_ vah_error() call gets turned into a one-line if body, then
the setenv() call starts to be part of the -A2 context of the vah_error.
 But I think you can work around that by creative newlines, as in:

    if ...
        vah_error(...);

    if (setenv("IFS",
               " \t\n", 1) != 0) {

or even creative comments:

    if ...
        vah_error(...);

    /* ensure the traditional IFS setting */
    if (setenv("IFS", " \t\n", 1) != 0) {

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