Thanks for the feedback.


>I'm happy with the general principle, but the method you have used has >the effect of changing LOTS of literal strings in the code. That makes >large amounts of the translations out of date, when the messages have >not really changed at all.


Right. I did not think about that. I see you have translated texts, but I did not notice that they are used for log messages too. Knowing that keeping translations up-to-date is a pain in the b*, this rules out my suggestion.

>As an alternative, how about using some of the log-facility bits in the >first argument of my_syslog, and appending the extra text in the >my_syslog code.
>
>
>#define MS_TFTP LOG_LOCAL0
>#define MS_DHCP LOG_LOCAL1
>
>my_syslog(MS_DHCP | LOG_WARNING, _("Ignoring domain %s for DHCP host >name %s"), config_domain, hostname);
>
>in my_syslog()
>
>if (level | MS_DHCP)
>    { extratext = "DHCP";
>           level &= ~MS_DHCP;
>    }
>
>etc..


OK.

OTOH, changing the strings will make the logs more concise...
Only a few, and it adds lots of copies of the string "DHCP:" to the
binary. The ones which are obviously more concise could  be changed.
Yes. I noticed some inconsistencies but not enough to warrant a complete overhaul.

>Also, I suspect that there are quite a few people doing pattern-matching >on the log strings, maybe it would be less likely to break that if the >sub-function was added to the ident:
 >
>Feb 27 07:38:41 fw dnsmasq-dhcp[29780]: DHCPREQUEST(lan-1) 192.168.10.44 >00:3F:56:20:11:f1


This will break those that select by the program name, for example in
syslog-ng. Changing anything in the log will likely break someone
somewhere. Olaf's proposal might be less intrusive in this respect.

Ah! That makes it easy for Olaf to achieve his original aim, assuming he
uses sylog-ng. This change is less obviously right, the extra-bits in
the syslog argument method can almost as easily be used to append to the
actual message. That's pretty non-negotiable, but I'm happy to append or
change the id, as people think best.


As a matter of fact, changing the program name works better in my case. We do all the logging straight to /var/log/messages and then use regex's to separate. Using program names dnsmasq, dnsmasq-dhcp and dnsmasq-tftp makes that a trivial exercise. Much easier than first filtering on dnsmasq and than identifying logs with and without DHCP:


How about this:

- Add to dnsmasq.h
#define MS_TFTP LOG_LOCAL0
#define MS_DHCP LOG_LOCAL1

- Add an option --log-separation which is disabled by default.

- Modify the relevant my_syslog calls to add MS_TFTP or MS_DHCP

- Modify log.c, to (optionally) include -dhcp and -tftp, something like this:

   if (!log_to_file)
       p += sprintf(p, "<%d>", LOG_PRI(priority) | log_fac);

if ((daemon->options2 & OPT_LOG2_LOG_SEPARATION) && (priority & MS_TFTP)) p += sprintf(p, "%.15s dnsmasq-tftp[%d]: ", ctime(&time_now) + 4, (int)pid); else if ((daemon->options2 & OPT_LOG2_LOG_SEPARATION) && (priority & MS_DHCP)) p += sprintf(p, "%.15s dnsmasq-dhcp[%d]: ", ctime(&time_now) + 4, (int)pid);
   else
p += sprintf(p, "%.15s dnsmasq[%d]: ", ctime(&time_now) + 4, (int)pid);


Notes: I think the options flagbits are all used since options in daemon is unsigned int, therefore options2 would be required. On further thought, using LOG_LOCAL0 collides with the OPT_DEBUG option, and possibly people using --log-facility
Better to use bits that are higher than LOG_NFACILITIES ?
Or even better to simply change the my_syslog to add another parameter?

void my_syslog(int priority, char *programid, const char *format, ...);
Where programid can be NULL or "tftp", "dhcp" ...


> If you're happy with that, Olaf, I'm happy to do the coding if you're
> happy to visit each my_syslog call and add the extra bits instead of the
> extra characters.....


No, no. This is my request/wish, so I have to put code where my mouth is ... ;-)



Olaf


Reply via email to