You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by be...@hyperreal.org on 1997/10/20 14:09:00 UTC
cvs commit: apachen/src/main alloc.c util_script.c
ben 97/10/20 05:09:00
Modified: src/main alloc.c util_script.c
Log:
Fix CGI under Win95 (plus some error checking). This needs doing more carefully.
Revision Changes Path
1.52 +18 -10 apachen/src/main/alloc.c
Index: alloc.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/alloc.c,v
retrieving revision 1.51
retrieving revision 1.52
diff -u -r1.51 -r1.52
--- alloc.c 1997/10/15 00:25:14 1.51
+++ alloc.c 1997/10/20 12:08:58 1.52
@@ -60,6 +60,7 @@
#include "httpd.h"
#include "multithread.h"
+#include "http_log.h"
#include <stdarg.h>
@@ -1217,9 +1218,9 @@
}
#ifdef WIN32
-#define enc_pipe(fds) _pipe(fds, 512, O_TEXT | O_NOINHERIT)
+#define os_pipe(fds) _pipe(fds, 512, O_BINARY | O_NOINHERIT)
#else
-#define enc_pipe(fds) pipe(fds)
+#define os_pipe(fds) pipe(fds)
#endif /* WIN32 */
/* for fdopen, to get binary mode */
@@ -1239,11 +1240,11 @@
int err_fds[2];
int save_errno;
- if (pipe_in && enc_pipe(in_fds) < 0) {
+ if (pipe_in && os_pipe(in_fds) < 0) {
return 0;
}
- if (pipe_out && enc_pipe(out_fds) < 0) {
+ if (pipe_out && os_pipe(out_fds) < 0) {
save_errno = errno;
if (pipe_in) {
close(in_fds[0]);
@@ -1253,7 +1254,7 @@
return 0;
}
- if (pipe_err && enc_pipe(err_fds) < 0) {
+ if (pipe_err && os_pipe(err_fds) < 0) {
save_errno = errno;
if (pipe_in) {
close(in_fds[0]);
@@ -1281,22 +1282,27 @@
/* Now do the right thing with your pipes */
if (pipe_in) {
hStdIn = dup(fileno(stdin));
- dup2(in_fds[0], fileno(stdin));
+ if(dup2(in_fds[0], fileno(stdin)))
+ aplog_error(APLOG_MARK, APLOG_ERR, NULL, "dup2(stdin) failed");
close(in_fds[0]);
}
if (pipe_out) {
hStdOut = dup(fileno(stdout));
- dup2(out_fds[1], fileno(stdout));
+ close(fileno(stdout));
+ if(dup2(out_fds[1], fileno(stdout)))
+ aplog_error(APLOG_MARK, APLOG_ERR, NULL, "dup2(stdout) failed");
close(out_fds[1]);
}
if (pipe_err) {
hStdErr = dup(fileno(stderr));
- dup2(err_fds[1], fileno(stderr));
+ if(dup2(err_fds[1], fileno(stderr)))
+ aplog_error(APLOG_MARK, APLOG_ERR, NULL, "dup2(stdin) failed");
close(err_fds[1]);
}
pid = (*func) (data);
- if (!pid) {
+ if (pid == -1) {
+ /* If we are going to save it, we ought to do something with it later, right? - Ben */
save_errno = errno;
close(in_fds[1]);
close(out_fds[0]);
@@ -1311,7 +1317,9 @@
if (pipe_err)
dup2(hStdErr, fileno(stderr));
- if (pid) {
+ if(pid == -1)
+ aplog_error(APLOG_MARK, APLOG_ERR, NULL, "spawn failed");
+ else {
note_subprocess(p, pid, kill_how);
if (pipe_in) {
*pipe_in = in_fds[1];
1.79 +6 -0 apachen/src/main/util_script.c
Index: util_script.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/util_script.c,v
retrieving revision 1.78
retrieving revision 1.79
diff -u -r1.78 -r1.79
--- util_script.c 1997/10/15 23:37:47 1.78
+++ util_script.c 1997/10/20 12:08:59 1.79
@@ -668,6 +668,7 @@
int i, sz;
char *dot;
char *exename;
+ char *s;
int is_exe = 0;
interpreter[0] = 0;
@@ -732,6 +733,11 @@
}
}
+ /* FIXME: Probably ought to do this in another buffer - Ben */
+ /* This really annoys me - Win95 (and not NT) spawn[vl]e don't like '/'! - Ben */
+ for(s=r->filename ; *s ; ++s)
+ if(*s == '/')
+ *s='\\';
if ((!r->args) || (!r->args[0]) || (ind(r->args, '=') >= 0)) {
if (is_exe || is_binary) {
Re: cvs commit: apachen/src/main alloc.c util_script.c
Posted by Ben Laurie <be...@algroup.co.uk>.
Paul Sutton wrote:
>
> On 20 Oct 1997 ben@hyperreal.org wrote:
> > ben 97/10/20 05:09:00
> >
> > Modified: src/main alloc.c util_script.c
> > Log:
> > Fix CGI under Win95 (plus some error checking). This needs doing more carefully.
>
> I rather wish that these had been committed separately.
So do I, but life is short and CVS wasn't working.
> There are several
> important bug fixes in this patch, and a couple of other changes. For
> reference, they are:
>
> 1. Fix CGI so it works on Windows 95 (with / -> \ translation)
> 2. Avoid "premature end-of-headers" error under CGI if the CGI cannot
> be spawned
> 3. Change the pipe mode from TEXT to BINARY
> 4. Close second argument to dup2() in one case - is this
> necessary?
No, that was left over from me thrashing around wildly trying to fix the
wrong problem.
> Incidently I'm not sure that patch for item 2 is correct. The
> spawn_child_err_core() function can still return -1, 0 or a PID from
> Windows. It should return 0 or a PID. A return value of -1 will be treated
> as a sucessful spawn by the parent, which is not correct.
Good point.
> Also if the
> return value is is 0 then the caller will log the spawn failure, so we
> don't need to do this within alloc.c (which is at a too low a level to log
> this anyway - it doesn't know the filename of the child process).
>
> All of this can be fixed with this can be done with this simple patch
> (from mail I sent about this bug on May 17th).
OK, clever clogs. So fix it.
Cheers,
Ben.
--
Ben Laurie |Phone: +44 (181) 735 0686|Apache Group member
Freelance Consultant |Fax: +44 (181) 735 0689|http://www.apache.org
and Technical Director|Email: ben@algroup.co.uk |Apache-SSL author
A.L. Digital Ltd, |http://www.algroup.co.uk/Apache-SSL
London, England. |"Apache: TDG" http://www.ora.com/catalog/apache
Re: cvs commit: apachen/src/main alloc.c util_script.c
Posted by Paul Sutton <pa...@ukweb.com>.
On 20 Oct 1997 ben@hyperreal.org wrote:
> ben 97/10/20 05:09:00
>
> Modified: src/main alloc.c util_script.c
> Log:
> Fix CGI under Win95 (plus some error checking). This needs doing more carefully.
I rather wish that these had been committed separately. There are several
important bug fixes in this patch, and a couple of other changes. For
reference, they are:
1. Fix CGI so it works on Windows 95 (with / -> \ translation)
2. Avoid "premature end-of-headers" error under CGI if the CGI cannot
be spawned
3. Change the pipe mode from TEXT to BINARY
4. Close second argument to dup2() in one case - is this
necessary?
Incidently I'm not sure that patch for item 2 is correct. The
spawn_child_err_core() function can still return -1, 0 or a PID from
Windows. It should return 0 or a PID. A return value of -1 will be treated
as a sucessful spawn by the parent, which is not correct. Also if the
return value is is 0 then the caller will log the spawn failure, so we
don't need to do this within alloc.c (which is at a too low a level to log
this anyway - it doesn't know the filename of the child process).
All of this can be fixed with this can be done with this simple patch
(from mail I sent about this bug on May 17th).
//pcs
*** alloc.c Sat May 17 04:50:18 1997
--- ../../apache_1_2b10_nt/src/alloc.c Sun May 25 13:46:46 1997
***************
*** 1096,1101 ****
--- 1096,1102 ----
}
pid = (*func)(data);
+ if (pid == -1) pid = 0;
if(!pid)
{
save_errno = errno;
Re: cvs commit: apachen/src/main alloc.c util_script.c
Posted by Paul Sutton <pa...@ukweb.com>.
On Mon, 20 Oct 1997, Ben Laurie wrote:
> > Incidently it would be nice if the patched code used the Apache style
> > guide.
>
> Hmmm ... if I knew in what way I'd erred, I'd fix it.
for(s=r->filename ; *s ; ++s)
if(*s == '/')
*s='\\';
should be
for (s = r->filename; *s; ++s)
if (*s == '/')
*s = '\\';
It's all in the style guide (http://dev.apache.org/styleguide.html).
//pcs
Re: cvs commit: apachen/src/main alloc.c util_script.c
Posted by Ben Laurie <be...@algroup.co.uk>.
Paul Sutton wrote:
>
> On 20 Oct 1997 ben@hyperreal.org wrote:
> > }
> > }
> > + /* FIXME: Probably ought to do this in another buffer - Ben */
> > + /* This really annoys me - Win95 (and not NT) spawn[vl]e don't like '/'! - Ben */
> > + for(s=r->filename ; *s ; ++s)
> > + if(*s == '/')
> > + *s='\\';
> >
> > if ((!r->args) || (!r->args[0]) || (ind(r->args, '=') >= 0)) {
> > if (is_exe || is_binary) {
>
> Ben, have you found this necessary now? When Ambarish's first NT port came
> to light I noticed that my Win95 machines would not run CGI without
> exactly this patch, but you could run CGI fine:
>
> | Date: Sun, 25 May 1997 14:18:08 +0100 (BST)
> | From: Ben Laurie <be...@gonzo.ben.algroup.co.uk>
> | Reply-To: dev-nt@apache.org
> | To: dev-nt@apache.org
> | Subject: Re: [BUG] spawning children on Win95
> |
> | Paul Sutton wrote:
> | > I couldn't get CGI to work on Windows 95... the reason seems to be that
> | > spawn*() functions do not understand forward slashes (/) in the pathname.
>
> | Hmmm ... I've got CGI working on Win95, without changes. But you are right,
> | / _used_ to work fine instead of \ everywhere, but now some functions need it.
> | Irritating, isn't it?
> |
> | I suppose the way you are doing your CGIs must be different from me...
>
> This could've have been fixed way back in May, though I guess that
> perhaps MS have been fiddling with the system calls in service packs
> and IE4 "upgrades".
Strange isn't it? I'm afraid I don't remember the history, but anyway,
if you needed / converted to \ then that's what you needed. It ain't my
fault you didn't add a patch.
I wonder - the critical difference in my test scenario is that I was
using all local disk (back in May? whenever), but now I'm using a
network mount.
> Incidently it would be nice if the patched code used the Apache style
> guide.
Hmmm ... if I knew in what way I'd erred, I'd fix it.
Cheers,
Ben.
--
Ben Laurie |Phone: +44 (181) 735 0686|Apache Group member
Freelance Consultant |Fax: +44 (181) 735 0689|http://www.apache.org
and Technical Director|Email: ben@algroup.co.uk |Apache-SSL author
A.L. Digital Ltd, |http://www.algroup.co.uk/Apache-SSL
London, England. |"Apache: TDG" http://www.ora.com/catalog/apache
Re: cvs commit: apachen/src/main alloc.c util_script.c
Posted by Paul Sutton <pa...@ukweb.com>.
On 20 Oct 1997 ben@hyperreal.org wrote:
> }
> }
> + /* FIXME: Probably ought to do this in another buffer - Ben */
> + /* This really annoys me - Win95 (and not NT) spawn[vl]e don't like '/'! - Ben */
> + for(s=r->filename ; *s ; ++s)
> + if(*s == '/')
> + *s='\\';
>
> if ((!r->args) || (!r->args[0]) || (ind(r->args, '=') >= 0)) {
> if (is_exe || is_binary) {
Ben, have you found this necessary now? When Ambarish's first NT port came
to light I noticed that my Win95 machines would not run CGI without
exactly this patch, but you could run CGI fine:
| Date: Sun, 25 May 1997 14:18:08 +0100 (BST)
| From: Ben Laurie <be...@gonzo.ben.algroup.co.uk>
| Reply-To: dev-nt@apache.org
| To: dev-nt@apache.org
| Subject: Re: [BUG] spawning children on Win95
|
| Paul Sutton wrote:
| > I couldn't get CGI to work on Windows 95... the reason seems to be that
| > spawn*() functions do not understand forward slashes (/) in the pathname.
| Hmmm ... I've got CGI working on Win95, without changes. But you are right,
| / _used_ to work fine instead of \ everywhere, but now some functions need it.
| Irritating, isn't it?
|
| I suppose the way you are doing your CGIs must be different from me...
This could've have been fixed way back in May, though I guess that
perhaps MS have been fiddling with the system calls in service packs
and IE4 "upgrades".
Incidently it would be nice if the patched code used the Apache style
guide.
//pcs