This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/2] posix: New Linux posix_spawn{p} implementation

Adhemerval Zanella wrote:
+#define SIGALL_SET ((sigset_t *)(const unsigned long long [2]){ -1,-1 })

This shouldn't assume that _SIGSET_NWORDS is 2. And since the code is Linux-specific, you should be able to avoid casting to sigset_t *; just write a constant that evaluates to sigset_t * without a cast.

+#ifdef O_CLOEXEC
+# ifndef __ASSUME_PIPE2
+  if (__have_pipe2 >= 0)
+# endif
+    {
+      r = __pipe2 (pipe_fds, O_CLOEXEC);
+# ifndef __ASSUME_PIPE2
+      if (__have_pipe2 == 0)

This sort of code is hard to read. Instead, declare substitutes like this after you do your #includes:

  #ifndef __ASSUME_PIPE2
  # define __have_pipe2 1
  #ifndef O_CLOEXEC
  # define O_CLOEXEC 0

and let the rest of the code just use __have_pipe2 and O_CLOEXEC, without the forest of #ifdefs.

+struct posix_spawn_args {
+  int p[2];
+  sigset_t oldmask;
+  const char *path;

This is a file name, right? Not a path? If so, it should not be called "path".

+  int (*exec)(const char *, char *const *, char *const *);

Spaces between )(. Also, please check the indenting overall; it seemed a bit inconsistent in this file.

+      while (argv[argc++]);
+    while (write_not_cancel (p, &ret, sizeof ret) < 0);

Use "continue".

+      /* Construct an argument list for the shell.  */
+      char *new_argv[argc];

This can overflow the stack.

+		  /* Only signal errors for file descriptors out of range.  */

"Signal errors only for ..."

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]