[Dnsmasq-discuss] Patch to change dnsmasq logentries
Simon Kelley
simon at thekelleys.org.uk
Fri Feb 27 19:39:10 GMT 2009
Olaf Westrik wrote:
>
> 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" ...
>
None of the calls to my_syslog set any of the facility bits, just the
priority ones. That's why I suggested defining the MS_* flags as
facility flags - it makes sure that they don't clash with any of the
priority flags. By using LOG_PRI(priority) you'll strip off the extra
flags, so there's no need for an extra argument, in fact there won't be
any extra code added to the many my_syslog() calls, which is good.
For portability, try not to assume more than is really necessary from
/usr/include/syslog.h - not all systems have the same stuff in there.
You are right that the daemon->options word is all used, so you'll need
a new one, but I'm not sure that there's any need to make this
behavior optional.
>
> > 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 ... ;-)
>
Fine by me!,
Cheers,
Simon.
>
>
> Olaf
>
>
More information about the Dnsmasq-discuss
mailing list