Re: [PATCH v5 19/40] lib-httpd: add upload.sh

From: Christian Couder <christian.couder@gmail.com>
Date: Sep 14, 2017, 07:43 UTC
Message-ID: <CAP8UFD2-V_2UOfp3zrTf0bSX7Y7g66FMAfhWW3==_GNd7oG_zw@mail.gmail.com>
In-Reply-To: <xmqq3798xvns.fsf@gitster.mtv.corp.google.com>

On Thu, Aug 3, 2017 at 10:07 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 18 quoted lines

> Christian Couder <christian.couder@gmail.com> writes:
>
>> +OLDIFS="$IFS"
>> +IFS='&'
>> +set -- $QUERY_STRING
>> +IFS="$OLDIFS"
>> +
>> +while test $# -gt 0
>> +do
>> +    key=${1%=*}
>> +    val=${1#*=}
>
> When you see that ${V%X*} and ${V#*X} appear in a pair for the same
> variable V and same delimiter X, it almost always indicates a bug
> waiting to happen.
>
> What's the definition of "key" here?  A member of known set of short
> tokens, all of which consists only of alphanumeric, or something?

Yeah, the key can be only "sha1", "type", "size" or "delete" as can be seen later in the code.

> Even if you do not currently plan to deal with a value with '=' in
> it, it may be prudent to double '%' above (and do not double '#').

Yeah I agree. Thanks for spotting this!

> Style: indent your shell script with tabs.

Sure.

Show 10 quoted lines

>> +    case "$key" in
>> +     "sha1") sha1="$val" ;;
>> +     "type") type="$val" ;;
>> +     "size") size="$val" ;;
>> +     "delete") delete=1 ;;
>> +     *) echo >&2 "unknown key '$key'" ;;
>> +    esac
>
> Indent your shell script with tabs; case/esac and the labels used
> for each case arms all align at the same column.

Yeah, it will be properly indented in the version I will send soon.