On Wed, Jun 02, 2010 at 10:42:44PM +0200, Stefan Fritsch wrote: > The patch is at > > http://people.apache.org/~sf/per-module-loglevel-v4/ ,
This looks good to me. Kudos for taking on such a task. It's kind of hard to review the individual patches with fixes-on-fixes separated out, or the big patch which jumbles everything up. The use of an extra per-file static global pointer to the module index seems awkward at first but turns into a neat hack by the end of the story. I very much like the new "trace" levels, and junking all the module-specific logging stuff in mod_rewrite/ssl. Some very minor nitpicking: Can you rename "struct logcfg" to something a bit more namespace-safe, with an ap_ prefix at least? struct ap_log_config, ap_logconf? In ap_reset_module_loglevels(), is there any reason not to simplify: + for (i = 0; i < total_modules + DYNAMIC_MODULE_LIMIT; i++) + l->module_levels[i] = val; to: memset(l->module_levels, val, total_modules + DYNAMIC_MODULE_LIMIT); ? That's all I have... thanks and nice work! Regards, Joe
