Re: [PATCH] BZ# 18125: setcontext: Call exit, not _exit, after last linked context executes.

[Mike Frysinger <vapier at gentoo dot org>]

On 13 Mar 2015 17:15, Carlos O'Donell wrote:

just nits ... actual changes look fine

> +#ifndef PATH_MAX
> +# define PATH_MAX 4096
> +#endif
> +char filename[PATH_MAX];

considering you just strcpy from argv[1], why not do one of:
	- assign filename to argv[1] directly
	- use strdup
that avoids the static bounds ugliness

> +/* It is intended that this function does nothing.  */
> +static void
> +cf (void)
> +{
> +  printf ("called context function\n");
> +  return;
> +}

return is kind of pointless

> +  char buf[] = "Called exit function\n";

doesn't really matter, but const ?

> +  printf ("PASS: %s", buf);
> +  res = close (fd);
> +  if (res == -1)
> +    {
> +      printf ("FAIL: Failed to close test file.\n");
> +      exit (1);
> +    }

seems weird ot print PASS and then a FAIL ... maybe move the PASS after the 
close ?

> +  return;
> +}

pointless return

> +#define TEST_FUNCTION do_test (argc, argv)

this is the default

> +# We want to run the test program and see if secontext called
> +# exit() and wrote out the test file we specified.  If the
> +# test exits with a non-zero status this will fail because we
> +# are using `set -e`.
> +$test_pre $test $tempfile

quote the paths ?  you did everywhere else :)

> +# Look for resulting file.
> +if [ -e "$tempfile" ]; then
> +  echo "PASS: tst-setcontext2 ran exit() and created $tempfile"
> +  cleanup
> +  exit 0

your trap should kick in here, so the explicit cleanup should not be needed
-mike

Attachment: signature.asc
Description: Digital signature