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
