Re: [PATCH 2/5] pager: do not close fd 2 unnecessarily

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Rubén Justo <rjusto@xxxxxxxxx> writes:

>> >  static struct child_process pager_process;
>> >  static const char *pager_program;
>> > +static int old_fd2 = -1;
>> 
>> What does the magic number "-1" mean?
>
> Invalid fd.
>
>> We often use it to signal
>> "uninitialized", but then what are concrete "initialized" values
>> mean?  "We dup2()'ed something else to stderr/fd #2 but before doing
>> so we saved the original fd #2 away to this variable, so that we can
>> restore fd #2 by another dup2() of the value of this variable when
>> we declare that we are done with the standard error stream"?
>> 
>> But that does not look like what is happening here.
>>  ....
>> Equally unclear magic number "1" is used here.
>> 
>> This value is different from pager_process.in, and my earlier "we
>> are saving away" does not apply, either.
>
> It applies, in 3/5.

We need to be prepared to see a series chomped at an early stage and
it should still make sense.  If the series does not make sense when
you stop before applying patch 3, it is a strong sign that this step
and the next step can be separated and structured better.

Or perhaps if they are made into a single patch it makes more sense
and becomes easier to explain?






[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux