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?