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]

Date:   Thu, 30 Jul 2020 08:16:31 +0200
From:   Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To:     Dmitry Torokhov <dmitry.torokhov@...il.com>
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

[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 :-)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)