linux-kernel - Re: [PATCH 1/8] platform/x86: fujitsu-laptop: move backlight input device setup to a separate function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170330040406.GL19787@marvin.atrad.com.au>
Date:   Thu, 30 Mar 2017 14:34:06 +1030
From:   Jonathan Woithe <jwoithe@...t42.net>
To:     Darren Hart <dvhart@...radead.org>
Cc:     Micha?? K??pie?? <kernel@...pniu.pl>,
        Andy Shevchenko <andy@...radead.org>,
        platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/8] platform/x86: fujitsu-laptop: move backlight input
 device setup to a separate function

On Wed, Mar 29, 2017 at 12:54:15PM -0700, Darren Hart wrote:
> On Mon, Mar 20, 2017 at 10:32:17AM +0100, Micha?? K??pie?? wrote:
> > +
> > +	return error;
> 
> This return path could be cleaned up a bit:
> 
> 	error = input_register_device(input);
> 	if (error)
> 		input_free_device(input);
> 
> 	return error;
> 
> But, this driver uses this "error/return 0" pattern pretty consistently, whereas
> most of the kernel uses ret instead of error, and will return ret on success and
> failure, relying on it being 0 in the successful case. Over the whole driver,
> we'd save several lines with the conversion and be more consistent with the rest
> of the kernel. But, local consistency is important too. Jonathan, do you have a
> preference for this driver?

I have no strong preferences, except to say that clarity is important.  As I
eluded to a few minutes ago, I agree that there's scope to address error
handling and there is a case to be made for bringing it into line with the
rest of the kernel.  I think this can be addressed in a separate patch
series though.  The present series under consideration doesn't make the
situation any worse (it actually improves it in some places) and introduces
worthwhile changes.  As such I don't see that we gain anything by delaying
it in order to address what is, at the end of the day, a separate concern.

Regards
  jonathan