Joe Orton wrote:
3) we can restore the pre-2.2.4 behaviour on Win32 by doing something simple like the below, without having to mess with the APR procattr code or continue adding complexity to the log.c code?
-1 not acceptable, you are looping back to dangling pipes, lingering processes, non-running code, etc. 2.2.4 and prior code didn't even run (piped loggers) on win32, remember? Unix had it's own issues. The 2.2.4-and-prior code was not acceptable, the proposed hack is similarly unacceptable. It was flawed on so many levels, the proposed patch is as close as I can come to a comprise given the broken state of APR (due entirely to your myopic interpretation of the versioning policy, which I'm through arguing.) Understand that is a /proposal/, it's still going to take me a few days to resolve a test/aprpipe.c fault on win32 (one of three on win32, of which the other two are well-known waiting on apr 2.0). Actually about 8 last hours of work to fix that and vet this patch as thoroughly as the first rounds, but I'm distracted thurs/fri and I'll see how much attention I can apply to it before the weekend. I appreciate your desire to avoid "polluting" the unix path, but it's really senseless to optimize code that's only invoked once/ process, and incredibly destructive to keep altering the code paths as we saw over the past few months. Keep a single impl. We have it right in 1.3.0 and I've already flipped the 'apr 1.3 required' bits in httpd trunk. Remember this audit already had uncloaked a host of UNIX issues. One code path. If the proposed patch leaves dangling handles, it's because this is a draft that I haven't finished vetting, I just wanted to get it out there so folks would have a preview. The final code will be clean, a few redundant dups in a rarely executed code block, but clean of dangling pipes on any platform.
--- server/log.c (revision 583629) +++ server/log.c (working copy) @@ -265,6 +265,11 @@ apr_proc_t *procnew; apr_file_t *errfile;+#ifdef WIN32+ /* workaround for APR 1.2.x */ + dummy_stderr = 0; +#endif + if (((rc = apr_procattr_create(&procattr, p)) == APR_SUCCESS) && ((rc = apr_procattr_cmdtype_set(procattr, APR_SHELLCMD_ENV)) == APR_SUCCESS)
