Mailing-List: contact new-httpd-help@apache.org; run by ezmlm
Precedence: bulk
Reply-To: new-httpd@apache.org
From: Tim  Costello <timcostello@ozemail.com.au>
Message-Id: <200005170425.OAA12749@oznet15.ozemail.com.au>
To: new-httpd@apache.org
CC: 
Subject: Re: cvs commit: apache-2.0/src/os/win32 main_win32.c os.h service.c
 service.h
Date: Wed, May 17 2000 15:25:28 GMT+1100
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Good work with these changes - they should make Windows 9x users a bit more
happy! Comments about this patch follow.

First up, there were a couple of changes in this patch that had nothing to do
with service/startup/shutdown/restart/console stuff at all. The change to add 
a function prototype to http_config.h was good, but can it be a separate patch 
next time? 

One more serious change is the one below:
> Index: os.h
..
>   -#define HAVE_SENDFILE
>   +#define HAVE_SENDFILE_UNKNOWN

Why? I might be missing something, but it sure looks like this will cause NT 
never to use TransmitFile! ap_send_fd in http_protocol.c relies on 
HAVE_SENDFILE being defined before it attempts calling iol_sendfile.

>     2) Isolation can't be complete, we need to know when the mpm is
>        fully initialized.  A new pointer to a no-arg function returning
>        void is provided for this purpose, ap_mpm_init_complete.  It is
>        only called if overridden with a non-NULL value prior to invoking
>        apache_main.

Why can't this call happen from within ap_mpm_run (just before master_main is
called) in winnt.c? This would guarantee that it was called once per process
and only for the parent process. 

If it must be done from the post config hook, can the NULL assignments happen 
there too (ie. just after the function is called)? This way, all the 
"ap_mpm_init_complete = NULL;" stuff that goes on at the end of each function 
that's a candidate for being called through ap_mpm_init_complete can 
disappear. I don't like the way a function must know that it is going to be 
called through a pointer, and that it is also responsible for setting said
pointer to NULL before it returns. 

>   Index: winnt.c
..
>   +typedef void (CALLBACK *ap_completion_t)();

Can this go into a header file? The same code has been put into winnt.c, 
main_win32.c and service.c.

>   -#include "service.h"
>   +//#include "service.h"

Why not just delete this? Same question for calls to service_set_status, etc.
that were commented out. 

>   Index: service.c
..
>   +    /* Free the kernel library */
>   +    // Worthless, methinks, since it won't be reclaimed
>   +    // FreeLibrary(hkernel);

Why not just call it? It would be one line instead of three and correct. The
comments add no value here. 

>   +    /* We have to quit right here to avoid an invalid page fault */
>   +    // But, this is worth experimenting with!
>   +    return (globdat.exit_status);

Why is it necessary to quit (ie. why do you get an invalid page fault)? What 
is "quitting" in this context? What exactly is worth experimenting with? 

Tim

This message was sent through MyMail http://www.mymail.com.au