Re: [patch] give gpioctl.c some love

2015-08-22 Thread Marc Balmer
Am 22.08.15 um 02:22 schrieb Timo Buhrmester: I was troubleshooting a GPIO problem and came across usr.sbin/gpioctl/gpioctl.c. The code is well-written, but I figured it needs some love, readability-wise. It uses unnecessarily deep nesting while trying to stay witin 80 characters per line

Re: [patch] give gpioctl.c some love

2015-08-22 Thread Christos Zoulas
In article 55d841b5.7010...@msys.ch, Marc Balmer m...@msys.ch wrote: Am 22.08.15 um 11:20 schrieb Felix Deichmann: Am 22.08.2015 um 10:15 schrieb Marc Balmer: Imo, this patch does not solve a problem, but creates new ones, maybe... The constness that you sprinkle is definitely not needed,

Re: [patch] give gpioctl.c some love

2015-08-22 Thread Felix Deichmann
Am 22.08.2015 um 10:15 schrieb Marc Balmer: Imo, this patch does not solve a problem, but creates new ones, maybe... The constness that you sprinkle is definitely not needed, only makes the code cluttered imo. This constness is mainly adding pointers to const in function arguments and for

Re: [patch] give gpioctl.c some love

2015-08-22 Thread Marc Balmer
Am 22.08.15 um 11:20 schrieb Felix Deichmann: Am 22.08.2015 um 10:15 schrieb Marc Balmer: Imo, this patch does not solve a problem, but creates new ones, maybe... The constness that you sprinkle is definitely not needed, only makes the code cluttered imo. This constness is mainly adding

Re: [patch] give gpioctl.c some love

2015-08-22 Thread Timo Buhrmester
Imo, this patch does not solve a problem, but creates new ones, maybe... I wonder what problems you imagine that to be. Care to elaborate? The constness that you sprinkle is definitely not needed, only makes the code cluttered imo. It actually makes it easier to understand because it's

Re: [patch] give gpioctl.c some love

2015-08-22 Thread Robert Elz
Date:Sat, 22 Aug 2015 02:22:00 +0200 From:Timo Buhrmester fstd.l...@gmail.com Message-ID: 20150822002200.GA12379@frozen.localdomain With the possible exception of the added consts, I also don't like any of the suggested changes - I tend to write if (p) myself often,

Re: [patch] give gpioctl.c some love

2015-08-22 Thread Timo Buhrmester
But while most of the changes are just objectionable, this part is just plain funny (and sad...) | +static unsigned | +get_uint_or_die(const char *str, const char *name) | +{ | + int oerrno; | + char *ep; | + | + long lval = strtol(str, ep, 0); | + if (!*str || *ep)

Re: [patch] give gpioctl.c some love

2015-08-22 Thread Timo Buhrmester
Look at it again ... | + oerrno = errno, errno = 0; Oh, you're right, that should of course happen at the beginning of the function. Good catch, my bad.

Re: [patch] give gpioctl.c some love

2015-08-22 Thread Richard Neswold
Oops. I got the precedence wrong. Sorry for the noise. -- Rich On Aug 22, 2015, at 10:51 AM, Rich Neswold rich.nesw...@gmail.com wrote: On Aug 22, 2015, at 10:34 AM, Timo Buhrmester fstd.l...@gmail.com wrote: Look at it again ... | +oerrno = errno, errno = 0; Oh, you're right,

[patch] give gpioctl.c some love

2015-08-21 Thread Timo Buhrmester
I was troubleshooting a GPIO problem and came across usr.sbin/gpioctl/gpioctl.c. The code is well-written, but I figured it needs some love, readability-wise. It uses unnecessarily deep nesting while trying to stay witin 80 characters per line at tabstop 8; I refactored parts of it to