[LEDE-DEV] odhcpd: [PATCH 1/3] enable loglevel setting via envvar ODHCPD_LOG_LEVEL

[John Crispin]

On 27/09/2016 01:33, Karl Palsson wrote:
> 
> John Crispin <john at phrozen.org> wrote:
>>
>>
>> On 22/09/2016 19:32, Karl Palsson wrote:
>>> From: Karl Palsson <karlp at etactica.com>
>>>
>>> Currently the loglevel is hardcoded to LOG_WARNING, even though there is
>>> debug log messages.  Allow an env var to control the log threshold.
>>>
>>> Signed-off-by: Karl Palsson <karlp at etactica.com>
>>> ---
>>>  src/odhcpd.c | 16 +++++++++++++++-
>>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/odhcpd.c b/src/odhcpd.c
>>> index 74830ac..c51cfa1 100644
>>> --- a/src/odhcpd.c
>>> +++ b/src/odhcpd.c
>>> @@ -58,7 +58,21 @@ static void sighandler(_unused int signal)
>>>  int main()
>>>  {
>>>  	openlog("odhcpd", LOG_PERROR | LOG_PID, LOG_DAEMON);
>>> -	setlogmask(LOG_UPTO(LOG_WARNING));
>>> +	char *env_log_level = getenv("ODHCPD_LOG_LEVEL");
>>> +	int log_level = LOG_WARNING;
>>> +	if (env_log_level) {
>>> +		char *end;
>>> +		errno = 0;
>>> +		long temp = strtol(env_log_level, &end, 0);
>>> +		if (end == env_log_level || *end != '\0'
>>> +			|| ((temp == LONG_MIN || temp == LONG_MAX) && errno == ERANGE)
>>> +			|| (log_level > LOG_DEBUG) || log_level < LOG_EMERG) {
>>> +			syslog(LOG_ERR, "ODHCPD_LOG_LEVEL envvar was invalid");
>>> +		} else {
>>> +			log_level = temp;
>>> +		}
>>> +	}
>>> +	setlogmask(LOG_UPTO(log_level));
>>>  	uloop_init();
>>
>> this is pretty bloaty. i am also not sure if using an env var
>> is the right way to solve this.
> 
> sure, strol sucks. But that's "the" way of checking. I could use
> atoi, and just convert nulls to some sort of default maybe. As
> for an env var, sure, they suck too, I just copied procd, was
> trying to be consistent. I'm open to other suggestions. I don't
> _need_ the logging anymore, after adding more for the fixing the
> "ignore" problem, and with logging feasible, there's probably
> more debug that would be relevant, so this patch could be dropped
> entirely if it's too much of a headache.
> 
> Cheers,
> Karl P

being able to set the debug level is good but not like this. why not use
the normal config code path or a commandline option ?

	John