All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: David Aguilar <davvid@gmail.com>
Cc: "Git Mailing List" <git@vger.kernel.org>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH v6 3/5] difftool: avoid returning -1 to cmd_main() from run_dir_diff()
Date: Thu, 30 Sep 2021 17:12:22 -0700	[thread overview]
Message-ID: <xmqqmtnt8uxl.fsf@gitster.g> (raw)
In-Reply-To: <CAJDDKr41qzbUudAHgokRARK-kEG4araFwJ0UhLmJ4Pca=xFZRA@mail.gmail.com> (David Aguilar's message of "Thu, 30 Sep 2021 16:25:10 -0700")

David Aguilar <davvid@gmail.com> writes:

>> Among them, the return value from run_command_v_opt() eventually
>> come from wait_or_whine(), I think, so it may be generic -1 or
>> it may be WEXITSTATUS() of the child process.
>>
>> But I am not sure if this particular caller cares.  It is not
>
> The property I was trying to maintain is that we would forward the result
> from the child process in most situations, so we should try and forward
> the result from run_command_v_opt() whenever possible.

Oh, if it were the case, then I agree that !!ret would not be
sufficient.

I just had an impression that this particular caller of the
run_command_v_opt() did not care, as it did not special case -1
returned from the function as different from what came from
WEXITSTATUS().

> That suggests that Ævar's sug is better...
>
>     return (ret < 0) ? 1 : ret;

Yup, that would work, as long as exit status 1 from the external
diff tool is not all that special (in which case, we cannot tell
it from 1 that came from ret = error()).

Thanks.

  reply	other threads:[~2021-10-01  0:12 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-30 17:01 [PATCH v6 1/5] difftool: create a tmpdir path without repeated slashes David Aguilar
2021-09-30 17:01 ` [PATCH v6 2/5] difftool: add a missing space to the run_dir_diff() comments David Aguilar
2021-09-30 17:01 ` [PATCH v6 3/5] difftool: avoid returning -1 to cmd_main() from run_dir_diff() David Aguilar
2021-09-30 22:06   ` Junio C Hamano
2021-09-30 23:25     ` David Aguilar
2021-10-01  0:12       ` Junio C Hamano [this message]
2021-09-30 17:01 ` [PATCH v6 4/5] difftool: refactor dir-diff to write files using a helper function David Aguilar
2021-09-30 22:17   ` Junio C Hamano
2021-09-30 23:34     ` David Aguilar
2021-09-30 17:01 ` [PATCH v6 5/5] difftool: remove an unnecessary call to strbuf_release() David Aguilar
2021-09-30 21:45 ` [PATCH v6 1/5] difftool: create a tmpdir path without repeated slashes Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqmtnt8uxl.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.