Re: [PATCH 2/2] pager_in_use: make sure output is still going to pager

From: Per Cederqvist <cederp@opera.com>
Date: Aug 11, 2015, 07:48 UTC
Message-ID: <CAP=KgsSnHg_007oMXdHYQZQWwL4McUTS8ty4-MYe-=3QB8+d8Q@mail.gmail.com>
In-Reply-To: <20150810172448.GA20168@sigill.intra.peff.net>

On Mon, Aug 10, 2015 at 7:24 PM, Jeff King <peff@peff.net> wrote:

Show 17 quoted lines

> On Mon, Aug 10, 2015 at 06:38:10PM +0200, Johannes Schindelin wrote:
>
>> > +const char *pipe_id_get(int fd)
>> > +{
>> > +   static struct strbuf id = STRBUF_INIT;
>> > +   struct stat st;
>> > +
>> > +   if (fstat(fd, &st) < 0 || !S_ISFIFO(st.st_mode))
>> > +           return NULL;
>>
>> Just a quick note: it seems that this check is not really working on
>> Windows. I tested this by running this test case manually (because TTY
>> is not set on Windows):
>
> Yeah, I'm not too surprised. I'm guessing your st_ino for pipes are all
> just the same or something. Or maybe S_ISFIFO doesn't pass (we don't
> technically need it, I don't think, and could just drop that check).

If you remove the S_ISFIFO check, you probably need to include the st_dev field in the pipe id. Otherwise, you could be unlucky and redirect the output to a file that just happens to have the same inode number as the pager pipe. Remember, inode numbers are only unique within a certain st_dev.

    /ceder