Ramkumar Ramachandra <artag...@gmail.com> writes:

> In preparation for a later patch that creates $dotest/autostash in
> git-rebase.sh before anything else happens, don't assume that the
> presence of a $dotest directory implies the existence of the $next and
> $last files.  The check for the files is in a conditional anyway, but
> `cat` is executed on potentially non-existent files.  Suppress this
> error output.
> Signed-off-by: Ramkumar Ramachandra <artag...@gmail.com>
> ---
>  git-am.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> diff --git a/git-am.sh b/git-am.sh
> index c092855..88aa438 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -446,8 +446,8 @@ done
>  # If the dotest directory exists, but we have finished applying all the
>  # patches in them, clear it out.
>  if test -d "$dotest" &&
> -   last=$(cat "$dotest/last") &&
> -   next=$(cat "$dotest/next") &&
> +   last=$(cat "$dotest/last" 2>/dev/null) &&
> +   next=$(cat "$dotest/next" 2>/dev/null) &&
>     test $# != 0 &&
>     test "$next" -gt "$last"
>  then

If you are introducing "dotest exists but next/last may not be
present" as a valid new state, it probably should check the presence
and/or absence of them explicitly, but more importantly, it is a
good idea to move "test $# != 0" higher. Earlier it did not matter
because when $dotest existed, next/last were supposed to be there
and absence of them was an error codepath. Now, missing these files
is not an error but is a perfectly normal state, so checking what
can be checked more cheaply makes sense as a general code hygiene.

As you may already know, I am not taking a patch that is not meant
for 'master' to fix regressions in 1.8.3 at this point in the cycle
after -rc2; please hold onto this and other patches as they won't
stay in my mailbox.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to