[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