httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <>
Subject Re: Change ap_cfg_* API?
Date Sun, 27 Feb 2011 14:53:09 GMT

On 02/26/2011 09:36 PM, Jeff Trawick wrote:
> On Sat, Feb 26, 2011 at 2:44 PM, Stefan Fritsch <> wrote:
>> Hi,
>> looking at PR 50824, I have noticed that ap_cfg_getline() and
>> ap_cfg_getc() are rather limited in that they do not allow the caller
>> to distinguish between EOF and an error (e.g. line too long, IO
>> error). ap_cfg_getline returns 0 on success and 1 if there was EOF or
>> an error, ap_cfg_getc returns the read character or EOF.
>> For ap_cfg_getline, we could return apr_status_t instead. Since
>> APR_SUCCESS is 0, modules would continue to work unchanged. But well
>> behaved consumeres could check for errors.
>> For ap_cfg_getc, there is no easy way to fix it, but I haven't found
>> any user. Maybe we could change it to accept a pointer where the char
>> should be stored, and return apr_status_t, too. This would probably
>> affect only very few modules.
>> Another problem is that ap_configfile_t contains pointers for fgets-
>> and getc-like functions, but it lacks an ferror-like function that
>> allows to check for errors. This would also need to be changed. There
>> are two ways: Make the functions return apr_status_t or add a pointer
>> for an ferror-like function. The latter would be easier to handle for
>> modules but it would still require ap_pcfg_open_custom() to take an
>> additional parameter and it would cause some additional work to store
>> the error status somewhere.
>> Any opinions? Comments?
> I am in favor of the choice which changes the interface and avoids the
> need for a separate ferror-like function.  We can just use the APR
> interface as closely as possible for the analogous functions; the
> apr_file_t arg becomes either void * for the callback function or
> ap_configfile_t for the normal API (basically what you said above).
> APR_DECLARE(apr_status_t) apr_file_getc(char *ch, apr_file_t *thefile);
> AP_DECLARE(apr_status_t) ap_cfg_getc(char *ch, ap_configfile_t *cfp);
> struct ap_configfile_t {
>     apr_status_t (*getch) (char *ch, void *param);	    /**< a
> getc()-like function */




View raw message