[Top][All Lists]

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

Re: [Nano-devel] [PATCH] Replace fork with clone to share file descripto

From: Benno Schulenberg
Subject: Re: [Nano-devel] [PATCH] Replace fork with clone to share file descriptors between parent and child.
Date: Thu, 16 Aug 2018 19:40:16 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1

Op 12-08-18 om 18:51 schreef Marco Diego Aurélio Mesquita:
> The attached patch replaces the call to fork in execute_command with clone.
> This way, file descriptors can be shared between parent and child effectively
> fixing .

Thanks for the patch.  It does fix the problem.  But do you have any
idea why the fork() created a problem?  What was it doing wrong?

> A dup2 was removed since it was not having any effect.

Is the removal necessary to make your patch work?  If not, then
it should be a separate change.

Is the clone() function POSIX?  Because it does not seem to exist
on OpenBSD:

  text.c: In function 'execute_command':
  text.c:1206: warning: implicit declaration of function 'clone'
  text.c:1206: error: 'CLONE_FILES' undeclared (first use in this function)

On NetBSD and FreeBSD it compiles fine, though.  And it works too
on NetBSD.  (On FreeBSD I can't test, because I haven't figured out
how to get around the failure of linking against gettext.)

> +#define STACK_SIZE 65536
> +     /* Stack size for cloned process to pipe out the buffer. */

Why such a big stack?  How much space does send_data() need, do
you think?

> -     /* Send each line, except a final empty line. */

Why remove this comment?  It should still be valid.

> +     int *to_fd = ((int*)fds);
> +     const filestruct *line = cutbuffer != NULL ? cutbuffer : 
> openfile->fileage;
>       while (line != NULL && (line->next != NULL || line->data[0] != '\0')) {

Between declarations and code should be a blank line.

Apart from those cosmetic issues, your patch looks fine.
The commit message should explain more why the change is
needed, though, why the descriptors need to be shared.


Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]