Re: [PATCH 2/3] posix: execvpe cleanup

[Mike Frysinger <vapier at gentoo dot org>]

On 19 Feb 2016 16:05, Adhemerval Zanella wrote:
> +	  if (!*subp)

i think GNU style says this should be:
	if (*subp == NULL)

> +      /* And the file to execute.  */
> +      memcpy (buffer + (subp - p) + (subp > p), file, file_len + 1);

adding (subp > p) as 0/1 is weird.  maybe put !! in front of it so it's
clear what you meant ?

> +	  case EACCES:
> +	  /* Record the we got a 'Permission denied' error.  If we end
> +             up finding no executable we can use, we want to diagnose
> +             that we did find one but were denied access.  */

bad indentation -- should start w/tabs

> +	  case ETIMEDOUT:
> +          /* Some strange filesystems like AFS return even
> +             stranger error numbers.  They cannot reasonably mean
> +             anything else so ignore those, too.  */

same here

> +# define EXECVP(__file, __argv) execvp (__file, __argv)

do these macros really need __ prefixes ?

> +      error (EXIT_FAILURE, 0, "environment variable number overflow");

error writes to stderr.  our style is for tests to use printf w/stdout.

> +			   (char*)"--direct", (char*)"--restart", NULL };

space before the *.  comes up multiple times in the patch.

> +  for (int i=1; i < max_args; ++i)

should be spaces around the =.  comes up multiple times in the patch.
-mike

Attachment: signature.asc
Description: Digital signature