Patchwork [v2,5/5] posix: Use posix_spawn for wordexp

login
register
mail settings
Submitter Adhemerval Zanella Netto
Date Oct. 8, 2019, 5:41 p.m.
Message ID <3cd9e4da-d5e2-df89-81ea-54835b449661@linaro.org>
Download mbox | patch
Permalink /patch/34882/
State New
Headers show

Comments

Adhemerval Zanella Netto - Oct. 8, 2019, 5:41 p.m.
On 07/10/2019 16:33, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> diff --git a/posix/wordexp-test.c b/posix/wordexp-test.c
>> index 10a0768a6b..ef780b0a65 100644
>> --- a/posix/wordexp-test.c
>> +++ b/posix/wordexp-test.c
> 
>> -/* For each fork increment the fork count.  */
>> -static void
>> -register_fork (void)
>> -{
>> -  registered_forks++;
>> -}
> 
> It's a bit sad to see this testing go away.  It was originally added to
> catch command execution with WRDE_NOCMD.
> 
> On Linux, could you enter a PID namespace instead and check that the
> next PID has the expected value?
> 
> Carlos, you added this testing.  Do you have an opinion here?
> 
>> diff --git a/posix/wordexp.c b/posix/wordexp.c
>> index 22c6d18a9c..e1aafcaceb 100644
>> --- a/posix/wordexp.c
>> +++ b/posix/wordexp.c
> 
>> +static char **
>> +create_environment (void)
>> +{
>> +  size_t s = 0;
>> +
>> +  /* Calculate total environment size, including 'IFS' if is present.  */
>> +  for (char **ep = __environ; *ep != NULL; ep++, s++);
> 
> I would put s++ into the body of the for loop, for clarity.  Or give ep
> a wider scope and use s = ep -- __environ.

Ack, I moved s++ to main loop.

> 
>> +  /* Include final NULL pointer.  */
>> +  char **newenviron = malloc (s * sizeof (char*));
>> +  if (newenviron == NULL)
>> +    return NULL;
> 
> char* should be char *.  I don't see how this includes the final NULL?
> 
> Should we do all this work only if IFS= is actually present?  That is,
> skip all this for getenv ("IFS) == NULL?

Ack.

> 
>> +  /* Copy current environment excluding 'IFS', to make sure the subshell
>> +     doesn't field-split on our behalf. */
> 
> That comment should apply to the entire function, I think.

Ack.

> 
>> @@ -884,13 +898,13 @@ exec_comm (char *comm, char **word, size_t *word_length, size_t *max_length,
>>    size_t maxnewlines = 0;
>>    char buffer[bufsize];
>>    pid_t pid;
>> -  int noexec = 0;
>> +  bool noexec = false;
>>  
>>    /* Do nothing if command substitution should not succeed.  */
>>    if (flags & WRDE_NOCMD)
>>      return WRDE_CMDSUB;
>>  
>> -  /* Don't fork() unless necessary */
>> +  /* Don't posix_spawn() unless necessary */
> 
> GNU style doesn't use () after function names.

Ack.

I fixed your remarks, rebased against master to pick-up your changed to 
wordexp tests and used dynarray to construct the new environment (it 
simplifies a bit the creation on a new environment for the IFS case).

	* include/spawn.h (__posix_spawn_file_actions_addopen): New
	prototype.
	* posix/spawn_faction_addopen.c (posix_spawn_file_actions_addopen):
	Add internal alias.
	* posix/wordexp.c (create_environment, free_environment): New
	functions.
	(exec_comm_child, exec_comm): Use posix_spawn instead of fork+exec.
	* posix/wordexp-test.c: Use libsupport and remove atfork usage.
---
Florian Weimer - Oct. 9, 2019, 9:11 a.m.
Thanks for the updated patch.

* Adhemerval Zanella:

>  static const char *
>  at_page_end (const char *words)
>  {
>    const int pagesize = getpagesize ();
> -  char *start = mmap (0, 2 * pagesize, PROT_READ|PROT_WRITE,
> -		      MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
> +  char *start = xmmap (0, 2 * pagesize, PROT_READ | PROT_WRITE,
> +		       MAP_PRIVATE | MAP_ANONYMOUS, -1);
>  
> -  if (start == MAP_FAILED)
> -    return start;
> -
> -  if (mprotect (start + pagesize, pagesize, PROT_NONE))
> -    {
> -      munmap (start, 2 * pagesize);
> -      return MAP_FAILED;
> -    }
> +  xmprotect (start + pagesize, pagesize, PROT_NONE);

I believe you can use <support/next_to_fault.h> for that.

> +	if (strncmp (*ep, "IFS=", sizeof ("IFS=")-1) != 0)

Missing spaces around -.  In my opinion, you should just call strlen.
GCC will fold it to a constant.

>   /* pid is unset if posix_spawn fails, so it keep the original value

“pid is not set” or “pid is not updated”, I think.

Rest looks okay to me.

Thanks,
Florian
Adhemerval Zanella Netto - Oct. 9, 2019, 12:18 p.m.
On 09/10/2019 06:11, Florian Weimer wrote:
> Thanks for the updated patch.
> 
> * Adhemerval Zanella:
> 
>>  static const char *
>>  at_page_end (const char *words)
>>  {
>>    const int pagesize = getpagesize ();
>> -  char *start = mmap (0, 2 * pagesize, PROT_READ|PROT_WRITE,
>> -		      MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
>> +  char *start = xmmap (0, 2 * pagesize, PROT_READ | PROT_WRITE,
>> +		       MAP_PRIVATE | MAP_ANONYMOUS, -1);
>>  
>> -  if (start == MAP_FAILED)
>> -    return start;
>> -
>> -  if (mprotect (start + pagesize, pagesize, PROT_NONE))
>> -    {
>> -      munmap (start, 2 * pagesize);
>> -      return MAP_FAILED;
>> -    }
>> +  xmprotect (start + pagesize, pagesize, PROT_NONE);
> 
> I believe you can use <support/next_to_fault.h> for that.

Ack, I will change to use it.

> 
>> +	if (strncmp (*ep, "IFS=", sizeof ("IFS=")-1) != 0)
> 
> Missing spaces around -.  In my opinion, you should just call strlen.
> GCC will fold it to a constant.

Ack.

> 
>>   /* pid is unset if posix_spawn fails, so it keep the original value
> 
> “pid is not set” or “pid is not updated”, I think.

Ack.

> 
> Rest looks okay to me.
> 
> Thanks,
> Florian
>

Patch

diff --git a/include/spawn.h b/include/spawn.h
index 7fdd965bd7..4a0b1849da 100644
--- a/include/spawn.h
+++ b/include/spawn.h
@@ -11,6 +11,9 @@  __typeof (posix_spawn_file_actions_addclose)
 __typeof (posix_spawn_file_actions_adddup2)
   __posix_spawn_file_actions_adddup2 attribute_hidden;
 
+__typeof (posix_spawn_file_actions_addopen)
+  __posix_spawn_file_actions_addopen attribute_hidden;
+
 __typeof (posix_spawn_file_actions_destroy)
   __posix_spawn_file_actions_destroy attribute_hidden;
 
diff --git a/posix/spawn_faction_addopen.c b/posix/spawn_faction_addopen.c
index d5694ee4d7..4fd64bb005 100644
--- a/posix/spawn_faction_addopen.c
+++ b/posix/spawn_faction_addopen.c
@@ -25,9 +25,9 @@ 
 /* Add an action to FILE-ACTIONS which tells the implementation to call
    `open' for the given file during the `spawn' call.  */
 int
-posix_spawn_file_actions_addopen (posix_spawn_file_actions_t *file_actions,
-				  int fd, const char *path, int oflag,
-				  mode_t mode)
+__posix_spawn_file_actions_addopen (posix_spawn_file_actions_t *file_actions,
+				    int fd, const char *path, int oflag,
+				    mode_t mode)
 {
   struct __spawn_action *rec;
 
@@ -60,3 +60,5 @@  posix_spawn_file_actions_addopen (posix_spawn_file_actions_t *file_actions,
 
   return 0;
 }
+weak_alias (__posix_spawn_file_actions_addopen,
+	    posix_spawn_file_actions_addopen)
diff --git a/posix/wordexp-test.c b/posix/wordexp-test.c
index a4d8bcf1da..34836f6240 100644
--- a/posix/wordexp-test.c
+++ b/posix/wordexp-test.c
@@ -15,19 +15,18 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <sys/stat.h>
-#include <sys/types.h>
-#include <sys/mman.h>
+#include <wordexp.h>
+#include <stdio.h>
 #include <fcntl.h>
-#include <unistd.h>
 #include <pwd.h>
-#include <stdio.h>
-#include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
-#include <wordexp.h>
+#include <sys/mman.h>
+
 #include <libc-pointer-arith.h>
-#include <dso_handle.h>
+#include <array_length.h>
+#include <support/xunistd.h>
+#include <support/check.h>
 
 #define IFS " \n\t"
 
@@ -40,7 +39,7 @@  struct test_case_struct
   size_t wordc;
   const char *wordv[10];
   const char *ifs;
-} test_case[] =
+} static test_case[] =
   {
     /* Simple word- and field-splitting */
     { 0, NULL, "one", 0, 1, { "one", }, IFS },
@@ -213,8 +212,6 @@  struct test_case_struct
     { WRDE_SYNTAX, NULL, "`\\", 0, 0, { NULL, }, IFS },     /* BZ 18042  */
     { WRDE_SYNTAX, NULL, "${", 0, 0, { NULL, }, IFS },      /* BZ 18043  */
     { WRDE_SYNTAX, NULL, "L${a:", 0, 0, { NULL, }, IFS },   /* BZ 18043#c4  */
-
-    { -1, NULL, NULL, 0, 0, { NULL, }, IFS },
   };
 
 static int testit (struct test_case_struct *tc);
@@ -226,21 +223,19 @@  command_line_test (const char *words)
   wordexp_t we;
   int i;
   int retval = wordexp (words, &we, 0);
-  printf ("wordexp returned %d\n", retval);
+  printf ("info: wordexp returned %d\n", retval);
   for (i = 0; i < we.we_wordc; i++)
-    printf ("we_wordv[%d] = \"%s\"\n", i, we.we_wordv[i]);
+    printf ("info: we_wordv[%d] = \"%s\"\n", i, we.we_wordv[i]);
 }
 
-int
-main (int argc, char *argv[])
+static int
+do_test (int argc, char *argv[])
 {
-  const char *globfile[] = { "one", "two", "three", NULL };
+  const char *globfile[] = { "one", "two", "three" };
   char tmpdir[32];
   struct passwd *pw;
   const char *cwd;
   int test;
-  int fail = 0;
-  int i;
   struct test_case_struct ts;
 
   if (argc > 1)
@@ -253,21 +248,18 @@  main (int argc, char *argv[])
 
   /* Set up arena for pathname expansion */
   tmpnam (tmpdir);
-  if (mkdir (tmpdir, S_IRWXU) || chdir (tmpdir))
-    return -1;
-  else
-    {
-      int fd;
+  xmkdir (tmpdir, S_IRWXU);
+  TEST_VERIFY_EXIT (chdir (tmpdir) == 0);
 
-      for (i = 0; globfile[i]; ++i)
-	if ((fd = creat (globfile[i], S_IRUSR | S_IWUSR)) == -1
-	    || close (fd))
-	  return -1;
+  for (int i = 0; i < array_length (globfile); ++i)
+    {
+      int fd = xopen (globfile[i], O_WRONLY|O_CREAT|O_TRUNC,
+		      S_IRUSR | S_IWUSR);
+      xclose (fd);
     }
 
-  for (test = 0; test_case[test].retval != -1; test++)
-    if (testit (&test_case[test]))
-      ++fail;
+  for (test = 0; test < array_length (test_case); test++)
+    TEST_COMPARE (testit (&test_case[test]), 0);
 
   /* Tilde-expansion tests. */
   pw = getpwnam ("root");
@@ -281,8 +273,7 @@  main (int argc, char *argv[])
       ts.wordv[0] = pw->pw_dir;
       ts.ifs = IFS;
 
-      if (testit (&ts))
-	++fail;
+      TEST_COMPARE (testit (&ts), 0);
 
       ts.retval = 0;
       ts.env = pw->pw_dir;
@@ -292,8 +283,7 @@  main (int argc, char *argv[])
       ts.wordv[0] = "x";
       ts.ifs = IFS;
 
-      if (testit (&ts))
-	++fail;
+      TEST_COMPARE (testit (&ts), 0);
     }
 
   /* "~" expands to value of $HOME when HOME is set */
@@ -308,8 +298,7 @@  main (int argc, char *argv[])
   ts.wordv[1] = "/dummy/home/foo";
   ts.ifs = IFS;
 
-  if (testit (&ts))
-    ++fail;
+  TEST_COMPARE (testit (&ts), 0);
 
   /* "~" expands to home dir from passwd file if HOME is not set */
 
@@ -325,14 +314,13 @@  main (int argc, char *argv[])
       ts.wordv[0] = pw->pw_dir;
       ts.ifs = IFS;
 
-      if (testit (&ts))
-	++fail;
+      TEST_COMPARE (testit (&ts), 0);
     }
 
   puts ("tests completed, now cleaning up");
 
   /* Clean up */
-  for (i = 0; globfile[i]; ++i)
+  for (int i = 0; i < array_length (globfile); ++i)
     remove (globfile[i]);
 
   if (cwd == NULL)
@@ -341,26 +329,17 @@  main (int argc, char *argv[])
   chdir (cwd);
   rmdir (tmpdir);
 
-  printf ("tests failed: %d\n", fail);
-
-  return fail != 0;
+  return 0;
 }
 
 static const char *
 at_page_end (const char *words)
 {
   const int pagesize = getpagesize ();
-  char *start = mmap (0, 2 * pagesize, PROT_READ|PROT_WRITE,
-		      MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
+  char *start = xmmap (0, 2 * pagesize, PROT_READ | PROT_WRITE,
+		       MAP_PRIVATE | MAP_ANONYMOUS, -1);
 
-  if (start == MAP_FAILED)
-    return start;
-
-  if (mprotect (start + pagesize, pagesize, PROT_NONE))
-    {
-      munmap (start, 2 * pagesize);
-      return MAP_FAILED;
-    }
+  xmprotect (start + pagesize, pagesize, PROT_NONE);
 
   /* Includes terminating NUL.  */
   const size_t words_size = strlen (words) + 1;
@@ -395,7 +374,7 @@  testit (struct test_case_struct *tc)
   sav_we.we_offs = 3;
   we = sav_we;
 
-  printf ("Test %d (%s): ", ++tests, tc->words);
+  printf ("info: test %d (%s): ", ++tests, tc->words);
   fflush (NULL);
   const char *words = at_page_end (tc->words);
 
@@ -404,7 +383,7 @@  testit (struct test_case_struct *tc)
       /* initial wordexp() call, to be appended to */
       if (wordexp ("pre1 pre2", &we, tc->flags & ~WRDE_APPEND) != 0)
         {
-	  printf ("FAILED setup\n");
+	  printf ("info: FAILED setup\n");
 	  return 1;
 	}
     }
@@ -436,7 +415,7 @@  testit (struct test_case_struct *tc)
   if (bzzzt)
     {
       printf ("FAILED\n");
-      printf ("Test words: <%s>, need retval %d, wordc %Zd\n",
+      printf ("info: Test words: <%s>, need retval %d, wordc %Zd\n",
 	      tc->words, tc->retval, tc->wordc);
       if (start_offs != 0)
 	printf ("(preceded by %d NULLs)\n", start_offs);
@@ -468,9 +447,11 @@  testit (struct test_case_struct *tc)
   const int page_size = getpagesize ();
   char *start = (char *) PTR_ALIGN_DOWN (words, page_size);
 
-  if (munmap (start, 2 * page_size) != 0)
-    return 1;
+  xmunmap (start, 2 * page_size);
 
   fflush (NULL);
   return bzzzt;
 }
+
+#define TEST_FUNCTION_ARGV do_test
+#include <support/test-driver.c>
diff --git a/posix/wordexp.c b/posix/wordexp.c
index 6a6e3a8e11..ea1ada1d2a 100644
--- a/posix/wordexp.c
+++ b/posix/wordexp.c
@@ -25,33 +25,18 @@ 
 #include <libintl.h>
 #include <paths.h>
 #include <pwd.h>
-#include <signal.h>
 #include <stdbool.h>
 #include <stdio.h>
-#include <stdlib.h>
 #include <string.h>
 #include <sys/param.h>
-#include <sys/stat.h>
-#include <sys/time.h>
-#include <sys/types.h>
-#include <sys/types.h>
 #include <sys/wait.h>
 #include <unistd.h>
-#include <wchar.h>
 #include <wordexp.h>
-#include <kernel-features.h>
+#include <spawn.h>
 #include <scratch_buffer.h>
-
-#include <libc-lock.h>
 #include <_itoa.h>
-
-/* Undefine the following line for the production version.  */
-/* #define NDEBUG 1 */
 #include <assert.h>
 
-/* Get some device information.  */
-#include <device-nrs.h>
-
 /*
  * This is a recursive-descent-style word expansion routine.
  */
@@ -812,61 +797,76 @@  parse_arith (char **word, size_t *word_length, size_t *max_length,
   return WRDE_SYNTAX;
 }
 
+#define DYNARRAY_STRUCT        strlist
+#define DYNARRAY_ELEMENT       char *
+#define DYNARRAY_PREFIX        strlist_
+/* Allocates about 512/1024 (32/64 bit) on stack.  */
+#define DYNARRAY_INITIAL_SIZE  128
+#include <malloc/dynarray-skeleton.c>
+
 /* Function called by child process in exec_comm() */
-static inline void
-__attribute__ ((always_inline))
-exec_comm_child (char *comm, int *fildes, int showerr, int noexec)
+static pid_t
+exec_comm_child (char *comm, int *fildes, bool showerr, bool noexec)
 {
-  const char *args[4] = { _PATH_BSHELL, "-c", comm, NULL };
+  pid_t pid = -1;
 
-  /* Execute the command, or just check syntax? */
-  if (noexec)
-    args[1] = "-nc";
+  /* Execute the command, or just check syntax?  */
+  const char *args[] = { _PATH_BSHELL, noexec ? "-nc" : "-c", comm, NULL };
+
+  posix_spawn_file_actions_t fa;
+  /* posix_spawn_file_actions_init does not fail.  */
+  __posix_spawn_file_actions_init (&fa);
 
-  /* Redirect output.  */
-  if (__glibc_likely (fildes[1] != STDOUT_FILENO))
+  /* Redirect output.  For check syntax only (noexec being true), exec_comm
+     explicits sets fildes[1] to -1, so check its value to avoid a failure in
+     __posix_spawn_file_actions_adddup2.  */
+  if (fildes[1] != -1)
     {
-      __dup2 (fildes[1], STDOUT_FILENO);
-      __close (fildes[1]);
+      if (__glibc_likely (fildes[1] != STDOUT_FILENO))
+	{
+	  if (__posix_spawn_file_actions_adddup2 (&fa, fildes[1],
+						  STDOUT_FILENO) != 0
+	      || __posix_spawn_file_actions_addclose (&fa, fildes[1]) != 0)
+	    goto out;
+	}
+      else
+	/* Reset the close-on-exec flag (if necessary).  */
+	if (__posix_spawn_file_actions_adddup2 (&fa, fildes[1], fildes[1])
+	    != 0)
+	  goto out;
     }
-  else
-    /* Reset the close-on-exec flag (if necessary).  */
-    __fcntl (fildes[1], F_SETFD, 0);
 
   /* Redirect stderr to /dev/null if we have to.  */
-  if (showerr == 0)
+  if (!showerr)
+    if (__posix_spawn_file_actions_addopen (&fa, STDERR_FILENO, _PATH_DEVNULL,
+					    O_WRONLY, 0) != 0)
+      goto out;
+
+  struct strlist newenv;
+  strlist_init (&newenv);
+
+  bool recreate_env = getenv ("IFS") != NULL;
+  if (recreate_env)
     {
-      struct stat64 st;
-      int fd;
-      __close (STDERR_FILENO);
-      fd = __open (_PATH_DEVNULL, O_WRONLY);
-      if (fd >= 0 && fd != STDERR_FILENO)
-	{
-	  __dup2 (fd, STDERR_FILENO);
-	  __close (fd);
-	}
-      /* Be paranoid.  Check that we actually opened the /dev/null
-	 device.  */
-      if (__builtin_expect (__fxstat64 (_STAT_VER, STDERR_FILENO, &st), 0) != 0
-	  || __builtin_expect (S_ISCHR (st.st_mode), 1) == 0
-#if defined DEV_NULL_MAJOR && defined DEV_NULL_MINOR
-	  || st.st_rdev != __gnu_dev_makedev (DEV_NULL_MAJOR, DEV_NULL_MINOR)
-#endif
-	  )
-	/* It's not the /dev/null device.  Stop right here.  The
-	   problem is: how do we stop?  We use _exit() with an
-	   hopefully unusual exit code.  */
-	_exit (90);
+      for (char **ep = __environ; *ep != NULL; ep++)
+	if (strncmp (*ep, "IFS=", sizeof ("IFS=")-1) != 0)
+	  strlist_add (&newenv, *ep);
+      strlist_add (&newenv, NULL);
+      if (strlist_has_failed (&newenv))
+	goto out;
     }
 
-  /* Make sure the subshell doesn't field-split on our behalf. */
-  __unsetenv ("IFS");
+  /* pid is unset if posix_spawn fails, so it keep the original value
+     of -1.  */
+  __posix_spawn (&pid, _PATH_BSHELL, &fa, NULL, (char *const *) args,
+		 recreate_env ? strlist_begin (&newenv) : __environ);
 
-  __close (fildes[0]);
-  __execve (_PATH_BSHELL, (char *const *) args, __environ);
+  strlist_free (&newenv);
+
+out:
+  __posix_spawn_file_actions_destroy (&fa);
 
-  /* Bad.  What now?  */
-  abort ();
+  return pid;
 }
 
 /* Function to execute a command and retrieve the results */
@@ -884,13 +884,13 @@  exec_comm (char *comm, char **word, size_t *word_length, size_t *max_length,
   size_t maxnewlines = 0;
   char buffer[bufsize];
   pid_t pid;
-  int noexec = 0;
+  bool noexec = false;
 
   /* Do nothing if command substitution should not succeed.  */
   if (flags & WRDE_NOCMD)
     return WRDE_CMDSUB;
 
-  /* Don't fork() unless necessary */
+  /* Don't posix_spawn unless necessary */
   if (!comm || !*comm)
     return 0;
 
@@ -898,19 +898,15 @@  exec_comm (char *comm, char **word, size_t *word_length, size_t *max_length,
     return WRDE_NOSPACE;
 
  again:
-  if ((pid = __fork ()) < 0)
+  pid = exec_comm_child (comm, fildes, noexec ? false : flags & WRDE_SHOWERR,
+			 noexec);
+  if (pid < 0)
     {
-      /* Bad */
       __close (fildes[0]);
       __close (fildes[1]);
       return WRDE_NOSPACE;
     }
 
-  if (pid == 0)
-    exec_comm_child (comm, fildes, noexec ? 0 : flags & WRDE_SHOWERR, noexec);
-
-  /* Parent */
-
   /* If we are just testing the syntax, only wait.  */
   if (noexec)
     return (TEMP_FAILURE_RETRY (__waitpid (pid, &status, 0)) == pid
@@ -1091,7 +1087,7 @@  exec_comm (char *comm, char **word, size_t *word_length, size_t *max_length,
   /* Check for syntax error (re-execute but with "-n" flag) */
   if (buflen < 1 && status != 0)
     {
-      noexec = 1;
+      noexec = true;
       goto again;
     }
 
@@ -1143,26 +1139,9 @@  parse_comm (char **word, size_t *word_length, size_t *max_length,
 	      /* Go -- give script to the shell */
 	      if (comm)
 		{
-#ifdef __libc_ptf_call
-		  /* We do not want the exec_comm call to be cut short
-		     by a thread cancellation since cleanup is very
-		     ugly.  Therefore disable cancellation for
-		     now.  */
-		  // XXX Ideally we do want the thread being cancelable.
-		  // XXX If demand is there we'll change it.
-		  int state = PTHREAD_CANCEL_ENABLE;
-		  __libc_ptf_call (__pthread_setcancelstate,
-				   (PTHREAD_CANCEL_DISABLE, &state), 0);
-#endif
-
+		  /* posix_spawn already handles thread cancellation.  */
 		  error = exec_comm (comm, word, word_length, max_length,
 				     flags, pwordexp, ifs, ifs_white);
-
-#ifdef __libc_ptf_call
-		  __libc_ptf_call (__pthread_setcancelstate,
-				   (state, NULL), 0);
-#endif
-
 		  free (comm);
 		}