httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@gmail.com>
Subject Re: Change ap_cfg_* API?
Date Sat, 26 Feb 2011 20:36:20 GMT
On Sat, Feb 26, 2011 at 2:44 PM, Stefan Fritsch <sf@sfritsch.de> 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 */

Mime
View raw message