Re: [PATCH] mingw: emulate write(2) that fails with a EPIPE

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Dec 17, 2015, 09:39 UTC
Message-ID: <alpine.DEB.2.20.1512171037310.6483@virtualbox>
In-Reply-To: <xmqq7fkedyx7.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Wed, 16 Dec 2015, Junio C Hamano wrote:

Show 27 quoted lines

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> >  int mingw_fflush(FILE *stream);
> >  #define fflush mingw_fflush
> >  
> > +static inline ssize_t mingw_write(int fd, const void *buf, size_t len)
> > +{
> > +	ssize_t result = write(fd, buf, len);
> > +
> > +	if (result < 0 && errno == EINVAL && buf) {
> > +		/* check if fd is a pipe */
> > +		HANDLE h = (HANDLE) _get_osfhandle(fd);
> > +		if (GetFileType(h) == FILE_TYPE_PIPE)
> > +			errno = EPIPE;
> > +		else
> > +			errno = EINVAL;
> > +	}
> > +
> > +	return result;
> > +}
> > +
> > +#define write mingw_write
> > +
> 
> It strikes me a bit strange to see this inlined compared to what
> appears in the context.  Shouldn't the implementation be done in
> compat/mingw.c like all others?

My intuition (which I honestly did not verify using performance tests) was that write() is called *much* more often than, say, open(), and therefore I wanted to interfere as little as possible with the performance penalty. Hence the choice of an inlined function as opposed to a non-optimizable increment of the call chain.

If it bothers you a lot I will set aside time to perform performance tests.

Ciao, Dscho