Re: wait_reading_process_ouput hangs in certain cases (w/ patches)

[Matthias Dahl]

Hello Paul...

On 22/11/17 09:55, Paul Eggert wrote:

> This doesn't look right, as nbytes might be negative (indicating an error).

You are absolutely right. I initially pushed it one line up before the
carryover is added but at a late call decided to put it a bit further up
for clarity and missed the conditional below somehow (turning blind!?).

Sorry about that. I will pay more attention in the future...

> Please start with a summary line that doesn't contain so much detail.
> <=50 chars is good. No trailing period. "Fix
> wait_reading_process_output_hang", perhaps.

Ok. Thanks for clearing that up. It was hard to figure out what style is
requested for Emacs.

> Kind of a long name, no? Perhaps make it shorter, so that you can write
> something like this:
> 
>    uintmax_t nbytes_read0 = wait_proc ? wait_proc->infd_num_bytes_read : 0;
> 
> Even better, shorten the member name too: "nbytes_read" is easier to
> read than "infd_num_bytes_read".

Well... I will have to think about that one. I am a firm proponent of
descriptive names that convey exactly what a function or variable is
being used for -- even at the expense of the length of that name.

nbytes_read is, imho, too generic -- especially given the huge context
of wait_..., so a good descriptive name helps.

Like I said, I will think about something that is shorter -- but coming
up with this name was not as easy as it may seem because I naturally
aimed at something shorter as well.

> Please limit the program to 80 columns.

Ok, will do.

> Space before "(" in function calls.

Argh, sorry, missed that again. I am not used to that style.

> It's annoying that the code (and the comment!) is duplicated.

I absolutely agree but quite honestly I was convinced I would get
negative feedback if I did put it in a function of its own, given
how the rest of wait_... looked and in general. So I went with the
duplicated code for exactly that reason.

Quite happy to change that.

I will have updated patches available later this week. Thanks for
your great feedback, again.

So long,
Matthias

-- 
Dipl.-Inf. (FH) Matthias Dahl | Software Engineer | binary-island.eu