RFA: fix libiberty/pex-djgpp.c

RFA: fix libiberty/pex-djgpp.c

[Eli Zaretskii]

The current CVS version of pex-djgpp.c didn't compile.  The patch
below fixes the problems I spotted; please review it.


2005-05-11  Eli Zaretskii  <eliz@gnu.org>

	* pex-djgpp.c: Include string.h, fcntl.h, unistd.h, and
	sys/stat.h.
	(pex_init): Fix last argument to pex_init_common.
	(pex_djgpp_exec_child): Remove leading underscore from _open,
	_dup, _dup2, _close, and _spawnv/_spawnvp.  Replace `program',
	which is undeclared, with `executable', which was unused.  Remove
	unused variable `e'.  Fix casting of last arg to spawnv/spawnvp.
	(funcs): Add pex_djgpp_fdopenr.
	(pex_djgpp_fdopenr): New function; prototype added near the
	beginning.
	(pex_djgpp_wait): Declare arguments with ATTRIBUTE_UNUSED.

--- libiberty/pex-djgpp.c~0	2005-05-10 18:33:32.000000000 +0300
+++ libiberty/pex-djgpp.c	2005-05-11 21:53:20.000000000 +0300
@@ -29,6 +29,10 @@ extern int errno;
 #ifdef HAVE_STDLIB_H
 #include <stdlib.h>
 #endif
+#include <string.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/stat.h>
 #include <process.h>
 
 /* Use ECHILD if available, otherwise use EINVAL.  */
@@ -46,6 +50,7 @@ static long pex_djgpp_exec_child (struct
 static int pex_djgpp_close (struct pex_obj *, int);
 static int pex_djgpp_wait (struct pex_obj *, long, int *, struct pex_time *,
 			   int, const char **, int *);
+static FILE *pex_djgpp_fdopenr (struct pex_obj *, int, int);
 
 /* The list of functions we pass to the common routines.  */
 
@@ -57,7 +62,7 @@ const struct pex_funcs funcs =
   pex_djgpp_close,
   pex_djgpp_wait,
   NULL, /* pipe */
-  NULL, /* fdopenr */
+  pex_djgpp_fdopenr,
   NULL  /* cleanup */
 };
 
@@ -68,7 +73,7 @@ pex_init (int flags, const char *pname, 
 {
   /* DJGPP does not support pipes.  */
   flags &= ~ PEX_USE_PIPES;
-  return pex_init_common (flags, pname, tempbase, funcs);
+  return pex_init_common (flags, pname, tempbase, &funcs);
 }
 
 /* Open a file for reading.  */
@@ -119,46 +124,46 @@ pex_djgpp_exec_child (struct pex_obj *ob
 
   if (in != STDIN_FILE_NO)
     {
-      org_in = _dup (STDIN_FILE_NO);
+      org_in = dup (STDIN_FILE_NO);
       if (org_in < 0)
 	{
 	  *err = errno;
-	  *errmsg = "_dup";
+	  *errmsg = "dup";
 	  return -1;
 	}
-      if (_dup2 (in, STDIN_FILE_NO) < 0)
+      if (dup2 (in, STDIN_FILE_NO) < 0)
 	{
 	  *err = errno;
-	  *errmsg = "_dup2";
+	  *errmsg = "dup2";
 	  return -1;
 	}
-      if (_close (in) < 0)
+      if (close (in) < 0)
 	{
 	  *err = errno;
-	  *errmsg = "_close";
+	  *errmsg = "close";
 	  return -1;
 	}
     }
 
   if (out != STDOUT_FILE_NO)
     {
-      org_out = _dup (STDOUT_FILE_NO);
+      org_out = dup (STDOUT_FILE_NO);
       if (org_out < 0)
 	{
 	  *err = errno;
-	  *errmsg = "_dup";
+	  *errmsg = "dup";
 	  return -1;
 	}
-      if (_dup2 (out, STDOUT_FILE_NO) < 0)
+      if (dup2 (out, STDOUT_FILE_NO) < 0)
 	{
 	  *err = errno;
-	  *errmsg = "_dup2";
+	  *errmsg = "dup2";
 	  return -1;
 	}
-      if (_close (out) < 0)
+      if (close (out) < 0)
 	{
 	  *err = errno;
-	  *errmsg = "_close";
+	  *errmsg = "close";
 	  return -1;
 	}
     }
@@ -166,70 +171,68 @@ pex_djgpp_exec_child (struct pex_obj *ob
   if (errdes != STDERR_FILE_NO
       || (flags & PEX_STDERR_TO_STDOUT) != 0)
     {
-      int e;
-
-      org_errdes = _dup (STDERR_FILE_NO);
+      org_errdes = dup (STDERR_FILE_NO);
       if (org_errdes < 0)
 	{
 	  *err = errno;
-	  *errmsg = "_dup";
+	  *errmsg = "dup";
 	  return -1;
 	}
-      if (_dup2 ((flags & PEX_STDERR_TO_STDOUT) != 0 ? STDOUT_FILE_NO : errdes,
+      if (dup2 ((flags & PEX_STDERR_TO_STDOUT) != 0 ? STDOUT_FILE_NO : errdes,
 		 STDERR_FILE_NO) < 0)
 	{
 	  *err = errno;
-	  *errmsg = "_dup2";
+	  *errmsg = "dup2";
 	  return -1;
 	}
       if (errdes != STDERR_FILE_NO)
 	{
-	  if (_close (errdes) < 0)
+	  if (close (errdes) < 0)
 	    {
 	      *err = errno;
-	      *errmsg = "_close";
+	      *errmsg = "close";
 	      return -1;
 	    }
 	}
     }
 
-  status = (((flags & PEX_SEARCH) != 0 ? _spawnvp : _spawnv)
-	    (P_WAIT, program, (const char **) argv));
+  status = (((flags & PEX_SEARCH) != 0 ? spawnvp : spawnv)
+	    (P_WAIT, executable, (char * const *) argv));
 
   if (status == -1)
     {
       *err = errno;
-      *errmsg = ((flags & PEX_SEARCH) != 0) ? "_spawnvp" : "_spawnv";
+      *errmsg = ((flags & PEX_SEARCH) != 0) ? "spawnvp" : "spawnv";
     }
 
   if (in != STDIN_FILE_NO)
     {
-      if (_dup2 (org_in, STDIN_FILE_NO) < 0)
+      if (dup2 (org_in, STDIN_FILE_NO) < 0)
 	{
 	  *err = errno;
-	  *errmsg = "_dup2";
+	  *errmsg = "dup2";
 	  return -1;
 	}
-      if (_close (org_in) < 0)
+      if (close (org_in) < 0)
 	{
 	  *err = errno;
-	  *errmsg = "_close";
+	  *errmsg = "close";
 	  return -1;
 	}
     }
 
   if (out != STDOUT_FILE_NO)
     {
-      if (_dup2 (org_out, STDOUT_FILE_NO) < 0)
+      if (dup2 (org_out, STDOUT_FILE_NO) < 0)
 	{
 	  *err = errno;
-	  *errmsg = "_dup2";
+	  *errmsg = "dup2";
 	  return -1;
 	}
-      if (_close (org_out) < 0)
+      if (close (org_out) < 0)
 	{
 	  *err = errno;
-	  *errmsg = "_close";
+	  *errmsg = "close";
 	  return -1;
 	}
     }
@@ -237,16 +240,16 @@ pex_djgpp_exec_child (struct pex_obj *ob
   if (errdes != STDERR_FILE_NO
       || (flags & PEX_STDERR_TO_STDOUT) != 0)
     {
-      if (_dup2 (org_errdes, STDERR_FILE_NO) < 0)
+      if (dup2 (org_errdes, STDERR_FILE_NO) < 0)
 	{
 	  *err = errno;
-	  *errmsg = "_dup2";
+	  *errmsg = "dup2";
 	  return -1;
 	}
-      if (_close (org_errdes) < 0)
+      if (close (org_errdes) < 0)
 	{
 	  *err = errno;
-	  *errmsg = "_close";
+	  *errmsg = "close";
 	  return -1;
 	}
     }
@@ -268,8 +271,9 @@ pex_djgpp_exec_child (struct pex_obj *ob
 
 static int
 pex_djgpp_wait (struct pex_obj *obj, long pid, int *status,
-		struct pex_time *time, int done, const char **errmsg,
-		int *err)
+		struct pex_time *time, int done ATTRIBUTE_UNUSED,
+		const char **errmsg ATTRIBUTE_UNUSED,
+		int *err ATTRIBUTE_UNUSED)
 {
   int *statuses;
 
@@ -281,3 +285,11 @@ pex_djgpp_wait (struct pex_obj *obj, lon
 
   return 0;
 }
+
+/* Get a FILE pointer to read from a file descriptor.  */
+
+static FILE *
+pex_djgpp_fdopenr (struct pex_obj *obj ATTRIBUTE_UNUSED, int fd, int binary)
+{
+  return fdopen (fd, (binary ? "rb" : "rt"));
+}

Re: RFA: fix libiberty/pex-djgpp.c

[Ian Lance Taylor]

"Eli Zaretskii" <eliz@gnu.org> writes:

> The current CVS version of pex-djgpp.c didn't compile.  The patch
> below fixes the problems I spotted; please review it.

Thanks for doing this.  I did not have a DJGPP system to try this code
out on.  It was obviously in fairly bad shape.

You don't need pex_djgpp_fdopenr.  It will only be called if
pex_init_common is called with PEX_USE_PIPES, which should never
happen since pex_init in pex-djgpp.c clears PEX_USE_PIPES.

Otherwise, this patch is approved.  Please check it into the gcc
repository first, as usual with libiberty patches.

Thanks.

Ian

Re: RFA: fix libiberty/pex-djgpp.c

[Eli Zaretskii]

> Cc: gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org,
>    djgpp-workers@delorie.com
> From: Ian Lance Taylor <ian@airs.com>
> Date: 11 May 2005 16:28:30 -0400
> 
> You don't need pex_djgpp_fdopenr.  It will only be called if
> pex_init_common is called with PEX_USE_PIPES

It's not easy to figure out what is needed and in what conditions.  It
would be nice if this machinery were to be documented a bit more
methodically.  Right now, the information is scattered between
pex-common.h, include/libiberty.h and a few comments in pex-*.c files.

> Otherwise, this patch is approved.  Please check it into the gcc
> repository first, as usual with libiberty patches.

I don't think I have write access to the GCC repository, sorry.

Anyway, thanks for a timely review.

Re: RFA: fix libiberty/pex-djgpp.c

[Ian Lance Taylor]

"Eli Zaretskii" <eliz@gnu.org> writes:

> > Cc: gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org,
> >    djgpp-workers@delorie.com
> > From: Ian Lance Taylor <ian@airs.com>
> > Date: 11 May 2005 16:28:30 -0400
> > 
> > You don't need pex_djgpp_fdopenr.  It will only be called if
> > pex_init_common is called with PEX_USE_PIPES
> 
> It's not easy to figure out what is needed and in what conditions.  It
> would be nice if this machinery were to be documented a bit more
> methodically.  Right now, the information is scattered between
> pex-common.h, include/libiberty.h and a few comments in pex-*.c files.

Don't forget pexecute.txh.

It's true that I did not bother to fully document the internals.  I
don't anticipate that very many people will be writing new pex-*.c
files.

> > Otherwise, this patch is approved.  Please check it into the gcc
> > repository first, as usual with libiberty patches.
> 
> I don't think I have write access to the GCC repository, sorry.

I will check it in soon, then.  Thanks.

Ian

Re: RFA: fix libiberty/pex-djgpp.c

[Ian Lance Taylor]

Ian Lance Taylor <ian@airs.com> writes:

> > > Otherwise, this patch is approved.  Please check it into the gcc
> > > repository first, as usual with libiberty patches.
> > 
> > I don't think I have write access to the GCC repository, sorry.
> 
> I will check it in soon, then.  Thanks.

Committed now to gcc and src.

Ian

Re: RFA: fix libiberty/pex-djgpp.c

[Eli Zaretskii]

> Cc: djgpp-workers@delorie.com, gcc-patches@gcc.gnu.org,
>   gdb-patches@sourceware.org
> From: Ian Lance Taylor <ian@airs.com>
> Date: 11 May 2005 23:46:22 -0400
> 
> Committed now to gcc and src.

Thank you.

Re: RFA: fix libiberty/pex-djgpp.c

[Eli Zaretskii]

> Cc: djgpp-workers@delorie.com, gcc-patches@gcc.gnu.org,
>   gdb-patches@sourceware.org
> From: Ian Lance Taylor <ian@airs.com>
> Date: 11 May 2005 22:17:43 -0400
> 
> > It's not easy to figure out what is needed and in what conditions.  It
> > would be nice if this machinery were to be documented a bit more
> > methodically.  Right now, the information is scattered between
> > pex-common.h, include/libiberty.h and a few comments in pex-*.c files.
> 
> Don't forget pexecute.txh.

Yes, but what's missing from all of these places is the overview (or
an example) of how would one go about running a pipeline of processes
using these functions, and a guided walk through the orchestrated
operation of these functions during such a pipeline, which would
explain the importance of each one in the process.  The pieces are
there, but the glue is missing, and since there are so many pieces and
tiny details, one can easily get lost in them.