linux-kernel - Re: [PATCH v18 3/3] Input: new da7280 haptic driver

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: <20200730063404.GH1665100@dtor-ws>
Date:   Wed, 29 Jul 2020 23:34:04 -0700
From:   Dmitry Torokhov <dmitry.torokhov@...il.com>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Cc:     Roy Im <roy.im.opensource@...semi.com>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Brian Masney <masneyb@...tation.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        Lee Jones <lee.jones@...aro.org>, Luca Weiss <luca@...tu.xyz>,
        Maximilian Luz <luzmaximilian@...il.com>,
        Pascal PAILLET-LME <p.paillet@...com>,
        Rob Herring <robh@...nel.org>,
        Samuel Ortiz <sameo@...ux.intel.com>,
        Thierry Reding <thierry.reding@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Support Opensource <support.opensource@...semi.com>,
        linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-pwm@...r.kernel.org, Petr Mladek <pmladek@...e.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>
Subject: Re: [PATCH v18 3/3] Input: new da7280 haptic driver

On Thu, Jul 30, 2020 at 08:16:31AM +0200, Uwe Kleine-König wrote:
> [Adding vsprintf maintainers to Cc:]
> 
> Hello,
> 
> On Wed, Jul 29, 2020 at 10:06:53PM -0700, Dmitry Torokhov wrote:
> > On Wed, Jul 29, 2020 at 09:21:45AM +0200, Uwe Kleine-König wrote:
> > > Hello,
> > > 
> > > On Tue, Jul 28, 2020 at 11:36:38PM -0700, Dmitry Torokhov wrote:
> > > > > v9: 
> > > > > 	- Removed the header file and put the definitions into the c file.
> > > > > 	- Updated the pwm code and error logs with %pE
> > > > 
> > > > I believe the %pE is to format an escaped buffer, you probably want to
> > > > %pe (lowercase) to print errors. I am also not quite sure if we want to
> > > > use it in cases when we have non-pointer error, or we should stick with
> > > > %d as most of the kernel does.
> > > 
> > > compared with %d %pe is easier to understand as it emits "-ETIMEOUT"
> > > instead of "-110". And yes, %pE is wrong.
> > 
> > While I can see that symbolic name instead of a numeric constant might
> > be appealing, I do not believe that we want fragments like this with
> > endless conversions between integer and pointer errors:
> > 
> > 	if (haptics->const_op_mode == DA7280_PWM_MODE) {
> > 		haptics->pwm_dev = devm_pwm_get(dev, NULL);
> > 		if (IS_ERR(haptics->pwm_dev)) {
> > 			error = PTR_ERR(haptics->pwm_dev);
> > 			if (error != -EPROBE_DEFER)
> > 				dev_err(dev, "unable to request PWM: %pE\n",
> > 					ERR_PTR(error));
> > 			return error;
> > 		}
> > 
> > Maybe we should introduce something like '%de' for the integer error
> > case?
> 
> I suggested that some time ago with limited success, see
> https://lore.kernel.org/lkml/20200129115516.zsvxu56e6h7gheiw@pathway.suse.cz/
> .
> 
> > In the meantime I would prefer using %d when we have integer
> > error. We should not see these error messages anyway ;)
> 
> I don't agree. Error messages are supposed to be helpful and I prefer
> some casting between error pointer and error int over emitting bare
> numbers to the kernel log. (And additionally the uglyness might help to
> convince the vsprintf guys that %de might be a good idea after all :-)

Sorry, but I do not think that I want to introduce this pointless
casting just to sway printk folks. Either we have proper way of
formatting error codes, or we do not and we stick with what was working
since forever.

Thanks.

-- 
Dmitry