apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ryan Bloom <...@covalent.net>
Subject Re: [PATCH] Re: cvs commit: apr/threadproc/unix proc.c
Date Thu, 18 Oct 2001 20:02:25 GMT
On Thursday 18 October 2001 12:15 pm, Jeff Trawick wrote:
> Ryan Bloom <rbb@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.

>
> >                                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, 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.

> 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.

> As far as apr_wait_or_timeout() and apr_process_child_status(), please
> see my my previous patch for examples on how to reduce the amount of
> non-portable constructs in Apache.  I just don't want to muck with
> that at the moment.  Unlike in my previous patch, I did not choose to
> use the portable view of exit status whenever possible because I want
> to get APR fixed appropriately first (while allowing all MPMs to work
> correctly).

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.

Ryan

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

Mime
View raw message