Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
Message-ID: <3E414F90.4030002@attglobal.net>
Date: Wed, 05 Feb 2003 12:53:20 -0500
From: Jeff Trawick <trawick@attglobal.net>
Reply-To: trawick@attglobal.net
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
 rv:1.2b) Gecko/20021016
MIME-Version: 1.0
To: dev@apr.apache.org
Subject: Re: [PATCH] letting the app do something useful when
  apr_proc_create()
 fails in the child process
References: <5.2.0.9.2.20030205093325.02b7f828@pop3.rowe-clan.net>
In-Reply-To: <5.2.0.9.2.20030205093325.02b7f828@pop3.rowe-clan.net>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

William A. Rowe, Jr. wrote:

> At 08:59 AM 2/5/2003, Jeff Trawick wrote:
>
> >Any concerns, particularly with respect to how the app determines if 
> the feature is available?  I think it would be fine to make the 
> feature always available but to simply note that on some platforms the 
> application-specified error function would never be called.
>
>
> Right... because those platforms that don't 'react' to the child_errfn are
> *generally* the platforms that will return a failure result from invoking
> apr_proc_create.  So we don't really lose anything by introducing your
> new schema, and we have everything to gain.  ++1


I think that you, Justin, and I agree that there is no need for a 
feature test macro.  That's cool.

> But what about simply an apr_proc_create_ex() function that accepts
> the callback.  I suspect this might be cleaner, because it will be easier
> to find code that failed to provide such a callback (which is really a 
> bug,
> IMHO.)  Like socket_create we can drop the _ex from APR 1.0 and always
> add that arguement to socket_create.  The code will probably be easier
> to read, in any case.  This should be supported on all platforms, it's 
> simply
> a noop where fork() wasn't involved.  Either way, though...


I'd like to see some +1s for this from others.  I'm not sure that it is 
a bug not to have the function.  I'm not really opposed, I simply would 
choose to continue adding process attributes rather than more parameters.

> Please make the callback take the apr_status_t result, the apr_procattr_t
> that failed and a pool.  Let the caller format it into a message if 
> they like,
> rather than creating more language-specific strings within apr itself. 
>  The
> callback should be able to take anything back out of the 
> apr_procattr_t that
> it's interested in.  And they should be able to take their context 
> back out
> of the apr_procattr_t rather than another userdata field.  (Heck, they 
> could
> even create pool data if they needed.)


on passing apr_procattr_t:

apr_procattr_t is private, so we'd have to add accessor functions for 
that to work.  Otherwise, it isn't useful.

--/--

on not having a parameter for specifying userdata:

How do you get context back from apr_procattr_t?  By adding 
apr_procattr_userdata_[get|set]()?

The app could attach userdata to a pool and we'd document that the pool 
better be the one passed to apr_proc_create() since that is the one we'd 
be passing to the child error function.  (or the one passed to 
apr_procattr_create())

Somehow it seems simple to document that what you pass to 
apr_procattr_child_errfn() set is what you get in the error function. 
Though if we go with the added error function parameter to 
apr_proc_create_ex() then I see how it is important to avoid adding even 
another parameter.

--/--

on APR not providing a string which tells what type of processing failed:

With no string from APR, you don't know if, for example, the failure was 
EPERM because

a) permissions on working directory were bad
b) permissions on executable were bad
c) no permission to raise rlimits to specified value

A certain large class of users would really benefit from such 
information, even if it is not in their native language and they have to 
feed it into google.  (But surely if we'd eventually translate other APR 
strings then an infrastructure would be in place.)

--/--

Thanks,

Jeff