syslogd config patch

[Denys Vlasenko]

On Tuesday 05 April 2011 20:46, Sergey Naumov wrote:
> Version that supports local logging in syslog.conf is stable. I
> personally received request for my patch to be merged too. Now it is
> up to Denys to make a decision. I sent him my stable patch yesterday.
> Remote logging support in syslog.conf requires significant changes in
> syslogd applet because as for now facility/priority parsing is tightly
> incorporated into local logging branch.
> 
> Latest stable version is attached (it is the same version I sent about
> half a year ago). I had no issues with it except a patch for
> uclibc-0.9.31 is needed to prevent changing kernel messages facility
> to "user". But I do not use extension options such as circular buffer
> and so on. Log rotation is supported in my patch but I do not use it
> too so it was not widely tested but only during development phase.
> If I receive a confirmation that the patch to be incorporated into
> busybox I will perform additional testing.


log_to_shmem is called repeatedly on every rule match. Bug.


unused: #define cfgbuf bb_common_bufsiz1


+       uint8_t isReg;
...
+IF_FEATURE_SYSLOGD_CFG( \
+       logRule_t *pLogR; \
+)

isReg (register?) and pLogR (wtf?) are rather undescriptive names.


+       char *t;
+       char *sel; //pointer to the current selector
+       char *nsel; //pointer to the next selector
+       uint8_t fl_neg; //indicates that selector has negation (kern.!err)
+       uint8_t fl_eq; //indicates that selector has equality (kern.=err)
+       uint32_t facmap; //bitmap of enabled facilities
+       uint8_t primap; //bitmap of enabled priorities
+       uint8_t pri; //priority code
+       uint8_t i; //counter for iteration through facilities/priorities
+       // use double pointer to avoid checking whether head was initialized
+       logRule_t **ppLogR = &(G.pLogR);
+       logRule_t *pLogR;
+       const CODE *code;

More of the same. You need to explain the variables because you named them badly.


+               c_fac = find_fac_prio(LOG_FAC(pri) << 3, NULL, facilitynames);
+               c_pri = find_fac_prio(LOG_PRI(pri), NULL, prioritynames);
+               if (c_fac && c_pri) {
+                       snprintf(res20, 20, "%s.%s", c_fac->c_name, c_pri->c_name);
+               } else {

Why do you bother calculating c_pri in case c_fac is NULL?
Old code wasn't doing that.


Style violations (brace placement and trailing whitespace).
Also, the rest of the file uses mostly /* */ comments, not //.


Please review/test:

http://git.busybox.net/busybox/commit/?id=73ef15cf3894716c1393ed21dee6e6bb2cdbc90f

-- 
vda