On 02/26/2011 09:36 PM, Jeff Trawick wrote: > On Sat, Feb 26, 2011 at 2:44 PM, Stefan Fritsch <[email protected]> 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 */
+1 Regards Rüdiger
