You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Jeff Trawick <tr...@attglobal.net> on 2001/10/19 16:45:44 UTC

Re: [Fwd: Re: [PATCH] Re: cvs commit: apr/threadproc/unix proc.c]

(sorry for the forward; my sendmail got sick for a while and was
discarding messages handed to it by my pop client :( )

Greg Ames <gr...@remulak.net> writes:

> -------- Original Message --------
> Subject: Re: [PATCH] Re: cvs commit: apr/threadproc/unix proc.c
> Date: Thu, 18 Oct 2001 13:02:25 -0700
> From: Ryan Bloom <rb...@covalent.net>
> Reply-To: rbb@covalent.net
> Organization: Covalent Technologies
> To: Jeff Trawick <tr...@attglobal.net>
> CC: dev@apr.apache.org
> References: <20...@icarus.apache.org>
> <20...@koj.rkbloom.net>
> <m3...@rdu88-250-106.nc.rr.com>
> 
> On Thursday 18 October 2001 12:15 pm, Jeff Trawick wrote:
> > Ryan Bloom <rb...@covalent.net> writes:
> > > I am missing something here.  This patch requires us to use the
> > > non-portable W* macros in Apache.  That is wrong.
> >
> > This patch does not force an APR app to use any W* macros other than
> > WCOREDUMP(), and most apps don't care about that.
> 
> As I read the last patch that you posted, all of the W* functions are
> still
> being used in ap_process_child status.

addressed in previous note

> > >                                This patch does not help APR to be
> > > more portable, rather it allows programs that use APR to continue to use
> > > non-portable constructs to solve the problem.  I would fully expect that
> > > in order for this to be done correctly, we will need to modify
> > > ap_process_child_status in mpm_common.c to not use the W* functions.  If
> > > we do anything else, then we have not helped APR to be portable.
> >
> > We're trying to make an APR app more portable.  I certainly plead
> > guilty to allowing an APR app to use a non-portable construct when the
> > app considers it necessary.  I hardly invented the concept.  It is an
> > important feature of APR in various areas.
> 
> I am saying that I don't believe it is necessary here, 

do you want to represent exited-with-a-coredump in an APR-like manner?
I don't care to, but if you want to, suggest an interface whereby we
can represent that and other unforseen system-specific issues.

>  and if it is,
> then this
> implementation is not the correct one.  Every other place that APR
> exposes
> some native information, it is done by querying a structure.  This patch
> exposes
> a non-portable value as a part of the main-line function.  No program
> can
> ever use the native_exit_code variable and expect to be portable.  If we
> are
> going to introduce something like that, it should be returned by a
> function.

no problem... what interface do you suggest?

> > What this patch does, if I may offer my version of
> > motherhood-and-apple-pie, is increase the ability of an APR app to do
> > meaningful stuff portably when compared with current CVS.  It allows
> > an APR app to do non-portable stuff if it so chooses by making
> > available the native status code.
> 
> But the native status code is not what people would expect it to be. 
> The native
> status code should be what the program returned when it exited.  It
> should not
> be that code after it was munged by the OS.

your opinion :)  mine differs; apr_wait_t (definitely should be called
something else) is simply what information the os makes available
regarding the dead child process

we don't need the native status code to be what the program returned
when it exited, because we provide that in a separate parameter

> I am very much against changing the API of an APR function temporarily.
> Returning the native exit code is not the correct solution to this
> problem,
> because that code can't really be used in a portable app.  If the idea
> is to
> go to three variables in the function, and then move back to two, then
> just
> go straight to two.  If the idea is to stick with three forever, then
> -1.

no, the idea wasn't to go from 3 to 2; I think all three pieces of
information are necessary, but if you want to provide one of the
pieces of information via a different mechanism, then by all means
show us what you mean

maybe somebody else can jump in here and offer some fresh ideas; maybe
you (Ryan) would be able to find some time to come up with concrete 
suggestions for how to convey all the necessary information back to
the app in a politically-correct manner

we can't have an Apache beta until this stuff is resolved (I wouldn't
veto a beta if just the WCOREDUMP() functionality is missing, but
there must be a design in place to restore that in the short-term);
maybe that will be enough to bring in some more attention/ideas

-- 
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
       http://www.geocities.com/SiliconValley/Park/9289/
             Born in Roswell... married an alien...

Re: [Fwd: Re: [PATCH] Re: cvs commit: apr/threadproc/unix proc.c]

Posted by Ryan Bloom <rb...@covalent.net>.
On Friday 19 October 2001 07:45 am, Jeff Trawick wrote:

Expect a patch by Monday night.  I have in-laws in town this weekend.  :-)

> your opinion :)  mine differs; apr_wait_t (definitely should be called
> something else) is simply what information the os makes available
> regarding the dead child process
>
> we don't need the native status code to be what the program returned
> when it exited, because we provide that in a separate parameter
>
> > I am very much against changing the API of an APR function temporarily.
> > Returning the native exit code is not the correct solution to this
> > problem,
> > because that code can't really be used in a portable app.  If the idea
> > is to
> > go to three variables in the function, and then move back to two, then
> > just
> > go straight to two.  If the idea is to stick with three forever, then
> > -1.
>
> no, the idea wasn't to go from 3 to 2; I think all three pieces of
> information are necessary, but if you want to provide one of the
> pieces of information via a different mechanism, then by all means
> show us what you mean
>
> maybe somebody else can jump in here and offer some fresh ideas; maybe
> you (Ryan) would be able to find some time to come up with concrete
> suggestions for how to convey all the necessary information back to
> the app in a politically-correct manner
>
> we can't have an Apache beta until this stuff is resolved (I wouldn't
> veto a beta if just the WCOREDUMP() functionality is missing, but
> there must be a design in place to restore that in the short-term);
> maybe that will be enough to bring in some more attention/ideas

______________________________________________________________
Ryan Bloom				rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Re: [Fwd: Re: [PATCH] Re: cvs commit: apr/threadproc/unix proc.c]

Posted by "William A. Rowe, Jr." <wr...@covalent.net>.
From: "Jeff Trawick" <tr...@attglobal.net>
Sent: Friday, October 19, 2001 9:45 AM
> 
> > I am very much against changing the API of an APR function temporarily.
> > Returning the native exit code is not the correct solution to this
> > problem,
> > because that code can't really be used in a portable app.  If the idea
> > is to
> > go to three variables in the function, and then move back to two, then
> > just
> > go straight to two.  If the idea is to stick with three forever, then
> > -1.
> 
> no, the idea wasn't to go from 3 to 2; I think all three pieces of
> information are necessary, but if you want to provide one of the
> pieces of information via a different mechanism, then by all means
> show us what you mean

Jeff... what if we return the two bits, APRized.  Then offer an apr_os_foo 
result through an accessor?  This will help identify non-portable applications
(such as stacktrack/coredump uses) while avoiding the 'temptation' to write
broken code in apr?

Bill