Re: [Nano-devel] [PATCH] just use a wait() instead

[Marco Diego Aurélio Mesquita]

On Sun, Aug 19, 2018 at 3:38 PM, Benno Schulenberg <address@hidden> wrote:
>
> Op 19-08-18 om 17:28 schreef Marco Diego Aurélio Mesquita:
>> On Sun, Aug 19, 2018 at 10:07:03AM +0200, Benno Schulenberg wrote:
>>> Attached patch also seems to fix it, and is much smaller.
>>>
>>> Does this patch solve the problem for you too?
>>
>> This patch makes the problem not occur on my machine but this is not a
>> solution. With the added wait call, nano main process will wait before
>> finally starting updating the text.
>
> Yes, while lying with my eyes closed beside the lake, I realized that
> too.  :)
>

I deeply envy the possibility you have to think about how to fix a bug.

>> Without 3 processes we can
>> get stuck because IO buffers may get full. It is a rare but possible
>> condition.
>
> Not so very rare, because especially with very large files, I can
> imagine that the user will want to run the buffer through a grep
> or something to reduce the amount of data.  Anyway, I tested it
> with a million line file, and it got stuck.  But works fine with
> the attached version of the patch,
>

I'm glad you found a way to reproduce it. Thanks!

>> And now we have 5 solutions for this problem. I still prefer
>> the reshuffling one because it condenses important code toghether.
>
> But doesn't it create a zombie process too?  But then as a child
> of the external command instead of as a child of nano?
>

Don't know. Can you reproduce it?

>> The spell feature uses a temp file for now, but we could make
>> it work just like the filtering feature.
>
> But maybe the external speller doesn't like to get the text piped
> to it and wants a temp file instead?
>
>> -     if (wait(NULL) == -1)
>> +     if (should_pipe && waitpid(pid_of_out_process, NULL, 0) == -1)
>> +             nperror("wait");
>> +
>> +     if (waitpid(pid_of_command, NULL, 0) == -1)
>>               nperror("wait");
>
> There is no need to wait for a specific PID -- when 'should_pipe'
> is true, we must just wait for two children instead of one.
>

Nice! This one has my blessing if you want to commit it. Forget the
reshuffling for now, we can do it later if so desired.