> -----Original Message----- > From: Joe Orton > Sent: Freitag, 4. Juni 2010 13:29 > To: dev@httpd.apache.org > Subject: Re: Per-module / per-dir loglevel configuration version 4 > > 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);
Hm, module_levels is int[] and memset works byte wise. So IMHO we would only touch the first total_modules + DYNAMIC_MODULE_LIMIT bytes of module_levels whereas module_levels is sizeof(int) * (total_modules + DYNAMIC_MODULE_LIMIT) large. Furthermore the values set will be IMHO wrong as each byte is filled with the same value and sizeof(int) bytes will be intepreted as an int later. Regards Rüdiger
