Re: [PROPOSAL] cgi_exec_info_t: detached addrspace fields combined
At 04:17 AM 6/28/2004, Joe Orton wrote: OK, the apr_procattr_addrspace_set() interface is sufficient to solve this problem, right? And there's no issue with back-porting that to the APR 0.9 branch? The only issue is how to use that interface from mod_cgi/the Netware MPM without requiring an httpd major MMN bump? So why not just overload the 'detached' field in cgi_exec_info_t inside mod_cgi/Netware MPM? That's should cause too much damange. Agreed, if that's a private structure, no bumps needed. -1 on overloading apr_procattr_detach_set() to do this, that's bad API design, and uglifying the APR API just to satisfy an httpd binary compat requirement just seems very wrong. I agree. If you want to overload the bit flags, that's fine, because the actual apr_procattr_t should be opaque to the user, correct? But having two seperate fn's to set detached v.s. addrspace seems like a much better public API.
Re: [PROPOSAL] cgi_exec_info_t: detached addrspace fields combined
OK, the apr_procattr_addrspace_set() interface is sufficient to solve this problem, right? And there's no issue with back-porting that to the APR 0.9 branch? The only issue is how to use that interface from mod_cgi/the Netware MPM without requiring an httpd major MMN bump? So why not just overload the 'detached' field in cgi_exec_info_t inside mod_cgi/Netware MPM? That's should cause too much damange. -1 on overloading apr_procattr_detach_set() to do this, that's bad API design, and uglifying the APR API just to satisfy an httpd binary compat requirement just seems very wrong. If the interface in APR HEAD is sufficient to solve the problem there's no reason why this is an APR 1.0 showstopper either. joe
Re: [PROPOSAL] cgi_exec_info_t: detached addrspace fields combined
OK, the apr_procattr_addrspace_set() interface is sufficient to solve this problem, right? And there's no issue with back-porting that to the APR 0.9 branch? The only issue is how to use that interface from mod_cgi/the Netware MPM without requiring an httpd major MMN bump? So why not just overload the 'detached' field in cgi_exec_info_t inside mod_cgi/Netware MPM? That's should cause too much damange. -1 on overloading apr_procattr_detach_set() to do this, that's bad API design, and uglifying the APR API just to satisfy an httpd binary compat requirement just seems very wrong. It does. If the interface in APR HEAD is sufficient to solve the problem there's no reason why this is an APR 1.0 showstopper either. Right. The issue is more that if we *don't* include an api change that is subsequently back-ported to apr 0.9 then we will have functionality in apr 0.9 and 1.1 but not in 1.0 - which just seems plain wrong and dumb... That's why I stopped the process. david
Re: [PROPOSAL] cgi_exec_info_t: detached addrspace fields combined
Joe is correct. The interface that we have now in APR HEAD is sufficient to solve the problem. If we released APR 1.0 now the same solution would exist in APR 1.0 and 1.1. The real issue is what to do about APR 0.9 since it will continue to live as long as applications like httpd 2.0 is dependent on it. The way I see it we have a few options. 1. Do nothing which includes *not* backporting the log.c patch (yes, I know. This is an issue for the [EMAIL PROTECTED] list) 2. Backport the apr_procattr_addrspace_set() interface currently in APR HEAD and make a call on wheither or not it deserves an MMN bump. 3. Adopt Jean-Jacques's proposal for both APR 1.0 and 0.9. 4. Implement the apr_procattr_detach_set() overloading solution in 0.9 and live with the difference between 0.9 and 1.0. I would rather see option #2 since it seems to be the right thing to do for APR. But we still have to deal with that nasty MMN bump issue. Brad Brad Nicholes Senior Software Engineer Novell, Inc., the leading provider of Net business solutions http://www.novell.com David Reid [EMAIL PROTECTED] Monday, June 28, 2004 5:23:03 AM OK, the apr_procattr_addrspace_set() interface is sufficient to solve this problem, right? And there's no issue with back-porting that to the APR 0.9 branch? The only issue is how to use that interface from mod_cgi/the Netware MPM without requiring an httpd major MMN bump? So why not just overload the 'detached' field in cgi_exec_info_t inside mod_cgi/Netware MPM? That's should cause too much damange. -1 on overloading apr_procattr_detach_set() to do this, that's bad API design, and uglifying the APR API just to satisfy an httpd binary compat requirement just seems very wrong. It does. If the interface in APR HEAD is sufficient to solve the problem there's no reason why this is an APR 1.0 showstopper either. Right. The issue is more that if we *don't* include an api change that is subsequently back-ported to apr 0.9 then we will have functionality in apr 0.9 and 1.1 but not in 1.0 - which just seems plain wrong and dumb... That's why I stopped the process. david
Re: [PROPOSAL] cgi_exec_info_t: detached addrspace fields combined
On Mon, Jun 28, 2004 at 10:08:34AM -0600, Brad Nicholes wrote: Joe is correct. The interface that we have now in APR HEAD is sufficient to solve the problem. If we released APR 1.0 now the same solution would exist in APR 1.0 and 1.1. The real issue is what to do about APR 0.9 since it will continue to live as long as applications like httpd 2.0 is dependent on it. The way I see it we have a few options. ... 2. Backport the apr_procattr_addrspace_set() interface currently in APR HEAD and make a call on wheither or not it deserves an MMN bump. These two are not related. Interfaces added to the APR 0.9 branch must conform to the APR versioning rules (i.e. don't make backwards incompatible changes on the 0.9 branch): so there is no issue with adding apr_procattr_addrspace_set() to the 0.9 branch. Whether or not the httpd MMN needs bumping depends entirely on the changes you make to *httpd*, not APR. joe
[PROPOSAL] cgi_exec_info_t: detached addrspace fields combined
To replace the addrspace field that was added in the cgi_exec_info_t
struct in mod_cgi.hI will like to propose extendingthe use of the
detached (apr_int32_t) field in cgi_exec_info_t and apr_procattr_t structs.
Currently that field is set to 0 by default and1 if the process to be
created will be a detached one.
What I will like to do to is to addan address space option to set
the second bit of that field in ordertoflag the newprocess to load
or not in a new address space.
The detached field in cgi_exec_info_t is set to 0 for all platforms and is used
only in mod_netware and mod_win32.
The changes will need to be back ported in order for NetWare to work after
log.c r r1.145 is back ported.Follow a partialpatch, I will certainly like to have suggestions:
Thank you,
Index: modules/arch/netware/mod_netware.c===RCS file: /home/cvs/httpd-2.0/modules/arch/netware/mod_netware.c,vretrieving revision 1.18diff -u -r1.18 mod_netware.c--- modules/arch/netware/mod_netware.c14 Jun 2004 17:28:25 -1.18+++ modules/arch/netware/mod_netware.c24 Jun 2004 19:07:12 -@@ -153,12 +154,12 @@ /* Run in its own address space if specified */ if(apr_table_get(d-file_handler_mode, ext))- e_info-addrspace = 1;+e_info-detached |= APR_PROC_NEWADDRSPACE; } /* Tokenize the full command string into its arguments */ apr_tokenize_to_argv(*cmd, (char***)argv, p);- e_info-detached = 1;+ e_info-detached |= APR_PROC_DETACHED; /* The first argument should be the executible */ *cmd = ap_server_root_relative(p, *argv[0]);Index: modules/generators/mod_cgi.c===RCS file: /home/cvs/httpd-2.0/modules/generators/mod_cgi.c,vretrieving revision 1.164diff -u -r1.164 mod_cgi.c--- modules/generators/mod_cgi.c14 Jun 2004 17:28:25 -1.164+++ modules/generators/mod_cgi.c24 Jun 2004 16:57:13 -@@ -432,8 +432,6 @@ ((rc = apr_procattr_detach_set(procattr, e_info-detached)) != APR_SUCCESS) ||- ((rc = apr_procattr_addrspace_set(procattr,- e_info-addrspace)) != APR_SUCCESS) || ((rc = apr_procattr_child_errfn_set(procattr, cgi_child_errfn)) != APR_SUCCESS)) { /* Something bad happened, tell the world. */ ap_log_rerror(APLOG_MARK, APLOG_ERR, rc, r,@@ -790,7 +788,6 @@ e_info.bb = NULL; e_info.ctx = NULL; e_info.next = NULL;- e_info.addrspace = 0; /* build the command line */ if ((rv = cgi_build_command(command, argv, r, p, e_info)) != APR_SUCCESS) {@@ -1058,7 +1055,6 @@ e_info.bb = bb; e_info.ctx = ctx; e_info.next = f-next;- e_info.addrspace = 0; if ((rv = cgi_build_command(command, argv, r, r-pool, e_info)) != APR_SUCCESS) {Index: modules/generators/mod_cgi.h===RCS file: /home/cvs/httpd-2.0/modules/generators/mod_cgi.h,vretrieving revision 1.14diff -u -r1.14 mod_cgi.h--- modules/generators/mod_cgi.h14 Jun 2004 17:28:25 -1.14+++ modules/generators/mod_cgi.h24 Jun 2004 16:54:08 -@@ -31,7 +31,6 @@ apr_bucket_brigade **bb; include_ctx_t *ctx; ap_filter_t *next;- apr_int32_t addrspace;} cgi_exec_info_t;/**RCS file: /home/cvspublic/apr/include/apr_thread_proc.h,vretrieving revision 1.110diff -u -r1.110 apr_thread_proc.h--- include/apr_thread_proc.h15 Jun 2004 20:51:25 -1.110+++ include/apr_thread_proc.h24 Jun 2004 19:06:59 -@@ -64,6 +64,10 @@ APR_PROC_SIGNAL_CORE = 4 /** process exited and dumped a core file */} apr_exit_why_e;+#define APR_NOT_DETACHED_NEWADDRSPACE 0 +#define APR_PROC_DETACHED 1+#define APR_PROC_NEWADDRSPACE 2 /* load the process in new address space */+/** did we exit the process */#define APR_PROC_CHECK_EXIT(x) (x APR_PROC_EXIT)/** did we get a signal */@@ -515,16 +519,6 @@ */APR_DECLARE(apr_status_t) apr_procattr_error_check_set(apr_procattr_t *attr, apr_int32_t chk);--/**- * Determine if the child should start in its own address space or using the - * current one from its parent- * @param attr The procattr we care about. - * @param addrspace Should the child start in its own address space? Default- * is no on NetWare and yes on other platforms.- */-APR_DECLARE(apr_status_t) apr_procattr_addrspace_set(apr_procattr_t *attr,- apr_int32_t addrspace);#if APR_HAS_FORK/**Index: include/arch/netware/apr_arch_threadproc.h===RCS file: /home/cvspublic/apr/include/arch/netware/apr_arch_threadproc.h,vretrieving revision 1.4diff -u -r1.4 apr_arch_threadproc.h--- include/arch/netware/apr_arch_threadproc.h14 Jun 2004 17:26:19 -1.4+++ include/arch/netware/apr_arch_threadproc.h24 Jun 2004 16:53:51 -@@ -60,7 +60,6 @@ char *currdir; apr_int32_t cmdtype; apr_int32_t detached;- apr_int32_t addrspace;};struct apr_thread_once_t {Index: threadproc/netware/proc.c===RCS file: /home/cvspublic/apr/threadproc/netware/proc.c,vretrieving revision 1.29diff -u -r1.29 proc.c--- threadproc/netware/proc.c14 Jun 2004 17:26:19
