[lxc-devel] Possible bug in lxc-netstat w/ patch; feature patch

Wed Jun 26 23:14:15 UTC 2013

Regarding the second patch, my thought is that if a user forgets the 
'--', the current behaviour will either crash as described in my bug 
report, or try to use the following arguments as the name of the 
container to open. That is, I believe that

         lxc-netstat -n <container-name> -n -a

will result in trying to do netstat on container "-a". I think my patch 
brings lxc-netstat's behaviour into a more consistent form, given that 
it seems

         lxc-netstat -n <container-name> -a -n

will result in calling netstat on container-name with "-a -n" as the 
arguments (see the last line of the case statement; line 81 of the 
unpatched source). The behaviour currently depends on the order of the 
arguments, as far as I can tell.

Perhaps a better approach would be to consistently die cleanly if no -- 
is provided, unless there are no further arguments? Or maybe it doesn't 
matter if behaviour is undefined under these circumstances.

--Andrew

On 06/26/2013 06:57 AM, Serge Hallyn wrote:
> Quoting Andrew Gilbert (andrewg800 at gmail.com):
>> Hi,
>> I think I've discovered a bug in lxc-netstat when trying to have it run
>> netstat with the '-n' option. I've attached a patch for this below
>> (patch 1). I've also attached a patch that tries to Do The Right Thing
>> if someone uses '-n' without the '--' before it (patch 2). I'd be happy
>> to resend the patches as separate emails if that would be better.
>> --Andrew G
> Hi,
>
> thanks, first patch confirmed and
>
> Acked-by: Serge E. Hallyn<serge.hallyn at ubuntu.com>
>
> from me.  But yes, please do resend as separate patches (threaded,
> 1/2 and 2/2) and (more importantly) do include a signed-off-by.
> (If there were more than 2 emails I'd suggest using git-send-email,
> but with two that's a bit over the top)
>
> (Question below on second patch)
>
>> S T E P S   T O   R E P R O D U C E:
>> 1. Start a container.
>> 2. Use lxc-netstat -n <container-name> -- -n -a to try to get numeric
>> IPs in netstat output.
>> 3. Get error "./lxc-netstat: 75: shift: can't shift that many."
>>
>> It appears that after the execution of lxc-unshare, the call to
>> lxc-netstat does not include the '--' separator to prevent the netstat
>> arguments being interpreted as lxc-netstat arguments. This only would
>> manifest as a bug if the option is one that is valid for lxc-netstat as
>> well as netstat, as the lone -n option is.
>>
>> P A T C H   1:
>>   From 78ea721f2a6b0cc01cffba93109bd8d8202eeb98 Mon Sep 17 00:00:00 2001
>> From: Andrew Gilbert<andrewg800 at gmail.com>
>> Date: Fri, 21 Jun 2013 11:24:37 -0700
>> Subject: [PATCH] Add double-dash to lxc-netstat re-call arguments
>>
>> When lxc-netstat was called by lxc-unshare, it would be given the
>> arguments intended for netstat from the first invocation, but without
>> anything to separate them from the arguments intended for lxc-netstat.
>> This meant that netstat arguments like -n would result in lxc-netstat
>> trying to process them
>> ---
>>    src/lxc/lxc-netstat.in | 2 +-
>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/lxc/lxc-netstat.in b/src/lxc/lxc-netstat.in
>> index 2fa2d23..229c214 100644
>> --- a/src/lxc/lxc-netstat.in
>> +++ b/src/lxc/lxc-netstat.in
>> @@ -93,7 +93,7 @@ if [ -z "$name" ]; then
>>    fi
>>
>>    if [ -z "$exec" ]; then
>> -    exec @BINDIR@/lxc-unshare -s MOUNT -- $0 -n $name --exec "$@"
>> +    exec @BINDIR@/lxc-unshare -s MOUNT -- $0 -n $name --exec -- "$@"
>>    fi
>>
>>    if lxc-info -n $name --state-is 'STOPPED'; then
>> -- 
>> 1.8.1.2
>>
>> P A T C H   2:
>>   From e45138027e08d291f4fad1357bcf5cb1b610c402 Mon Sep 17 00:00:00 2001
>> From: Andrew Gilbert<andrewg800 at gmail.com>
>> Date: Fri, 21 Jun 2013 14:36:28 -0700
>> Subject: [PATCH 2/2] Add -n differentiation to lxc-netstat
>>
>> lxc-netstat now only processes an -n argument if it has not previously
>> received a value for $name from --name or -n. If it _has_ received such
>> a value, it stops processing arguments and leaves the -n for netstat.
>> This does not apply to the use of --name after a name has been provided
>> by --name or -n; the current behaviour continues.
> Can you please give a test case here?  It sounds to me like you're
> supporting
>
> 	lxc-netstat -n <container-name> -n -a
>
> without the '--'.  I'm not sure we want to do that - the rules end
> up more ambiguous than a simple 'use -- to pass arguments on'.
>
>> ---
>>    src/lxc/lxc-netstat.in | 13 ++++++++++++-
>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/lxc/lxc-netstat.in b/src/lxc/lxc-netstat.in
>> index 229c214..4a239d9 100644
>> --- a/src/lxc/lxc-netstat.in
>> +++ b/src/lxc/lxc-netstat.in
>> @@ -66,12 +66,23 @@ get_parent_cgroup()
>>    }
>>
>>    exec=""
>> +name=""
>>
>>    while true; do
>>        case $1 in
>>            -h|--help)
>>                help; exit 1;;
>> -        -n|--name)
>> +        -n)
>> +            # If we already have a value for $name, treat -n as being an
>> +            # argument for netstat
>> +            if [ -n "$name" ]
>> +            then
>> +                break
>> +            else
>> +                name="$2"; shift 2;
>> +            fi
>> +            ;;
>> +        --name)
>>                name=$2; shift 2;;
>>            --exec)
>>                exec="exec"; shift;;
>> -- 
>> 1.8.1.2
>>
>>
>>
>> ------------------------------------------------------------------------------
>> This SF.net email is sponsored by Windows:
>>
>> Build for Windows Store.
>>
>> http://p.sf.net/sfu/windows-dev2dev
>> _______________________________________________
>> Lxc-devel mailing list
>> Lxc-devel at lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/lxc-devel