All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
	"git\@vger.kernel.org" <git@vger.kernel.org>,
	Johannes Sixt <j6t@kdbg.org>
Subject: Re: [PATCH] mingw: emulate write(2) that fails with a EPIPE
Date: Wed, 16 Dec 2015 12:36:21 -0800	[thread overview]
Message-ID: <xmqqmvtacere.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <CAPig+cQzCHtyVs47=ASs5V2BSV7gpVszbdTFydiRhnz3gtD2Qw@mail.gmail.com> (Eric Sunshine's message of "Wed, 16 Dec 2015 14:47:15 -0500")

Eric Sunshine <sunshine@sunshineco.com> writes:

>> +       if (result < 0 && errno == EINVAL && buf) {
>
> Here, errno is EINVAL...
>
>> +               /* check if fd is a pipe */
>> +               HANDLE h = (HANDLE) _get_osfhandle(fd);
>> +               if (GetFileType(h) == FILE_TYPE_PIPE)
>> +                       errno = EPIPE;
>> +               else
>> +                       errno = EINVAL;
>
> Does any of the code between the outer 'if' and this point clobber
> errno, or are you merely assigning EINVAL for robustness against
> future changes?

I do think it is a good idea to reassign 'errno' here to stress on
the fact that we return EPIPE only when we positively know that we
were reading from a pipe, and otherwise we won't change the original
errno at all.

But at the same time, if other things that are called before we
figure out if we were reading from a pipe could affect errno, I
wonder if that is an indication of a bug--an error return from
system functions that the code is not responding to.  For example,
can _get_osfhandle() fail and when it does what would we see?
Perhaps NULL in h?

  reply	other threads:[~2015-12-16 20:36 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-16 12:14 [PATCH] mingw: emulate write(2) that fails with a EPIPE Johannes Schindelin
2015-12-16 18:35 ` Junio C Hamano
2015-12-17  9:39   ` Johannes Schindelin
2015-12-17 16:40     ` Junio C Hamano
2015-12-17 17:08       ` Johannes Schindelin
2015-12-16 19:47 ` Eric Sunshine
2015-12-16 20:36   ` Junio C Hamano [this message]
2015-12-17  9:37   ` Johannes Schindelin
2015-12-17 17:08 ` [PATCH v2] " Johannes Schindelin
2015-12-17 19:22   ` Junio C Hamano
2015-12-18 16:10     ` Johannes Schindelin
2015-12-18 20:57   ` Johannes Sixt
2015-12-21 16:59     ` 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=xmqqmvtacere.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=sunshine@sunshineco.com \
    /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.