[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: popen bugs, popen_safer
From: |
Bruno Haible |
Subject: |
Re: popen bugs, popen_safer |
Date: |
Thu, 20 Aug 2009 10:14:03 +0200 |
User-agent: |
KMail/1.9.9 |
Hi Eric,
> I went ahead and implemented popen_safer.
Well done!
> +/* Get the original definition of popen. It might be defined as a macro. */
This is not needed. I'm not aware of any platform that defines popen as a
macro:
$ LC_ALL=C fgrep popen */usr/include/stdio.h
aix-4.3.2/usr/include/stdio.h:extern FILE *popen();
aix-4.3.2/usr/include/stdio.h:extern FILE *popen(const char *, const char
*);
aix-5.1.0/usr/include/stdio.h:extern FILE *popen();
aix-5.1.0/usr/include/stdio.h:extern FILE *popen(const char *, const char
*);
aix-5.3.0/usr/include/stdio.h:extern FILE *popen();
aix-5.3.0/usr/include/stdio.h:extern FILE *popen(const char *, const char
*);
beos/usr/include/stdio.h:FILE *popen(const char *cmd, const char *type);
cygwin/usr/include/stdio.h:FILE * _EXFUN(popen, (const char *, const char *));
freebsd-5.2.1/usr/include/stdio.h:FILE *popen(const char *, const char *);
freebsd-6.0/usr/include/stdio.h:FILE *popen(const char *, const char *);
glibc-2.3.6/usr/include/stdio.h:extern FILE *popen (__const char *__command,
__const char *__modes);
glibc-2.3.6/usr/include/stdio.h:/* Close a stream opened by popen and return
the status of its child.
haiku/usr/include/stdio.h:extern FILE *popen(const char *command,
const char *mode);
hpux-10.20/usr/include/stdio.h: extern FILE *popen(const char *, const char
*);
hpux-10.20/usr/include/stdio.h: extern FILE *popen();
hpux-11.00/usr/include/stdio.h: extern __FILE *popen(const char *, const
char *);
hpux-11.00/usr/include/stdio.h: extern FILE *popen();
hpux-11.11/usr/include/stdio.h:# pragma HP_DEFINED_EXTERNAL
getw,putw,pclose,popen,tempnam
hpux-11.11/usr/include/stdio.h:# pragma HP_NO_RELOCATION
getw,putw,pclose,popen,tempnam
hpux-11.11/usr/include/stdio.h:# pragma HP_LONG_RETURN
getw,putw,pclose,popen,tempnam
hpux-11.11/usr/include/stdio.h: extern __FILE *popen(const char *, const
char *);
hpux-11.11/usr/include/stdio.h: extern FILE *popen();
hpux-11.23-ia64/usr/include/stdio.h:# pragma extern getw, putw, pclose,
popen, tempnam
hpux-11.23-ia64/usr/include/stdio.h:# pragma HP_DEFINED_EXTERNAL
getw,putw,pclose,popen,tempnam
hpux-11.23-ia64/usr/include/stdio.h:# pragma HP_NO_RELOCATION
getw,putw,pclose,popen,tempnam
hpux-11.23-ia64/usr/include/stdio.h:# pragma HP_LONG_RETURN
getw,putw,pclose,popen,tempnam
hpux-11.23-ia64/usr/include/stdio.h: extern __FILE *popen(const char *,
const char *);
hpux-11.23-ia64/usr/include/stdio.h: extern FILE *popen();
interix-3.5/usr/include/stdio.h:FILE* __cdecl popen (const char *, const char
*);
irix-6.5/usr/include/stdio.h:extern FILE *popen(const char *, const char
*);
macosx-10.3/usr/include/stdio.h:FILE *popen(const char *, const char *);
mingw/usr/include/stdio.h:_CRTIMP FILE* __cdecl _popen (const char*, const
char*);
mingw/usr/include/stdio.h:_CRTIMP FILE* __cdecl popen (const char*, const
char*);
mingw/usr/include/stdio.h:_CRTIMP FILE* __cdecl _wpopen (const wchar_t*, const
wchar_t*);
mingw/usr/include/stdio.h:_CRTIMP FILE* __cdecl wpopen (const wchar_t*, const
wchar_t*);
mirbsd-10/usr/include/stdio.h:FILE *popen(const char *, const char *);
netbsd-3.0/usr/include/stdio.h:FILE *popen(const char *, const char *);
openbsd-3.8/usr/include/stdio.h:FILE *popen(const char *, const char *);
osf1-4.0d/usr/include/stdio.h:extern FILE *popen __((const char *, const
char *));
osf1-5.1a/usr/include/stdio.h:extern FILE *popen __((const char *, const
char *));
pips/usr/include/stdio.h:FILE *popen(const char *, const char *);
pips/usr/include/stdio.h:int popen3(const char *file, const char *cmd, char**
envp, int fds[3]);
qnx/usr/include/stdio.h:extern _CSTD FILE *popen( const char *__filename,
const char *__mode );
solaris-2.10/usr/include/stdio.h:extern FILE *popen(const char *, const char
*);
solaris-2.10/usr/include/stdio.h:extern FILE *popen();
> +#define __need_FILE
> +# include <stdio.h>
> +#undef __need_FILE
This looks as it could interfere with glibc headers. Can you use a
gnulib-defined
macro name, such as __need_system_stdio_h, instead?
> + if (cloexec0 < 0)
> + {
> + if (open ("/dev/null", O_RDONLY) != STDIN_FILENO
> + || fcntl (STDIN_FILENO, F_SETFD,
> + fcntl (STDIN_FILENO, F_GETFD) | FD_CLOEXEC) == -1)
> + abort ();
> + }
> + if (cloexec1 < 0)
> + {
> + if (open ("/dev/null", O_RDONLY) != STDOUT_FILENO
> + || fcntl (STDOUT_FILENO, F_SETFD,
> + fcntl (STDOUT_FILENO, F_GETFD) | FD_CLOEXEC) == -1)
> + abort ();
> + }
Maybe add some comments, what this code is trying to do? Took me a while to
understand it.
> diff --git a/tests/test-fopen.c b/tests/test-fopen.c
> index 337a389..b9e3694 100644
> --- a/tests/test-fopen.c
> +++ b/tests/test-fopen.c
> @@ -21,6 +21,10 @@
> #include <stdio.h>
> #include <stdlib.h>
>
> +#if GNULIB_FOPEN_SAFER
> +# include "stdio--.h"
> +#endif
> +
> #define ASSERT(expr) \
> do \
> {
> \
This is becoming confusing: Why should the 'fopen' test suddenly test
fopen_safer,
not fopen??
I would find it better to introduce a new test module fopen-safer-tests, that
shares
the bulk of the code with fopen-tests. File structure:
test-fopen.h : all common code, starting with the definition of ASSERT,
test-fopen.c : just the includes and #include "test-fopen.h"
test-fopen-safer.c : just the includes, then #include "stdio--.h", then
#include "test-fopen.h"
See test-snprintf-posix.h, test-snprintf-posix.c, test-vsnprintf-posix.c for
an example of this coding pattern.
Likewise for test-open.c and fcntl--.h.
Likewise for test-popen.c and stdio--.h.
> +#include "cloexec.h"
I don't see where cloexec.h is defined in your patch 3/3.
Bruno
- Re: fcntl module, (continued)
- Re: fcntl module, Bruno Haible, 2009/08/24
- [PATCH] Move more flags to lib/fcntl.in.h, Paolo Bonzini, 2009/08/20
- Re: [PATCH] Move more flags to lib/fcntl.in.h, Eric Blake, 2009/08/20
- Re: [PATCH] Move more flags to lib/fcntl.in.h, Bruno Haible, 2009/08/22
- Re: O_CLOEXEC support (was: popen bugs, popen_safer), Bruno Haible, 2009/08/22
- Re: O_CLOEXEC support, Paolo Bonzini, 2009/08/22
Re: popen bugs, popen_safer,
Bruno Haible <=