Messages in this thread | | | From | Alexandre Courbot <> | Date | Thu, 14 Feb 2013 12:00:54 +0900 | Subject | Re: [PATCH 1/4] gpiolib: check descriptors validity before use |
| |
On Thu, Feb 14, 2013 at 7:49 AM, Ryan Mallon <rmallon@gmail.com> wrote: > Is it really useful to use the same pr_debug for the error case? Why not do: > > desc = gpio_to_desc(gpio); > if (!desc) { > pr_debug("%s - Invalid gpio %d\n", __func__, gpio); > return -EINVAL; > } > > ... > > At this point desc is known valid, though you could just use the gpio > number that was passed in (assuming that it is always the same as > desc_to_gpio). > > pr_debug("%s: gpio%d status %d\n", __func__, > desc_to_gpio(desc), status); > return status; > > That provides more information (the original gpio number and the reason > for the -EINVAL) if the gpio is not valid, and removes the ugly ternary > operator from the pr_debug. Same goes for the other functions.
That's mainly a style issue - I tried to preserve the code's behavior as much as possible with these patches. But indeed the invalid GPIO case is special, and on second thought identifying invalid GPIOs are "gpio-1" is confusing. One could also say horrible.
Guess this deserves to be fixed in a v2. Thanks.
Alex.
| |