linux-kernel - Re: [PATCH v6 02/17] media: v4l2: async: Allow searching for asd of any type

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: <20180926063335.3c3b863d@coco.lan>
Date:   Wed, 26 Sep 2018 06:33:35 -0300
From:   Mauro Carvalho Chehab <mchehab+samsung@...nel.org>
To:     Steve Longerbeam <steve_longerbeam@...tor.com>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc:     Steve Longerbeam <slongerbeam@...il.com>,
        <linux-media@...r.kernel.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Niklas Söderlund 
        <niklas.soderlund+renesas@...natech.se>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Sebastian Reichel <sre@...nel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6 02/17] media: v4l2: async: Allow searching for asd of
 any type

Em Tue, 25 Sep 2018 18:05:36 -0700
Steve Longerbeam <steve_longerbeam@...tor.com> escreveu:

> On 09/25/2018 03:20 PM, Mauro Carvalho Chehab wrote:
> > Em Tue, 25 Sep 2018 14:04:21 -0700
> > Steve Longerbeam <steve_longerbeam@...tor.com> escreveu:
> >  
> >>>     
> >>>> @@ -392,12 +406,11 @@ static int __v4l2_async_notifier_register(struct v4l2_async_notifier *notifier)
> >>>>    		case V4L2_ASYNC_MATCH_CUSTOM:
> >>>>    		case V4L2_ASYNC_MATCH_DEVNAME:
> >>>>    		case V4L2_ASYNC_MATCH_I2C:
> >>>> -			break;
> >>>>    		case V4L2_ASYNC_MATCH_FWNODE:
> >>>> -			if (v4l2_async_notifier_fwnode_has_async_subdev(
> >>>> -				    notifier, asd->match.fwnode, i)) {
> >>>> +			if (v4l2_async_notifier_has_async_subdev(
> >>>> +				    notifier, asd, i)) {
> >>>>    				dev_err(dev,
> >>>> -					"fwnode has already been registered or in notifier's subdev list\n");
> >>>> +					"asd has already been registered or in notifier's subdev list\n");  
> >>> Please, never use "asd" on messages printed to the user. While someone
> >>> may understand it while reading the source code, for a poor use,
> >>> "asd" is just a random sequence of 3 characters.  
> >> I will change the message to read:
> >>
> >> "subdev descriptor already listed in this or other notifiers".  
> > Perfect!  
> 
> But the error message is removed in the subsequent patch
> "[PATCH 03/17] media: v4l2: async: Add v4l2_async_notifier_add_subdev".
> 
> I could bring it back as a dev_dbg() in v4l2_async_notifier_asd_valid(), but
> this shouldn't be a dev_err() anymore since it is up to the media platform
> to decide whether an already existing subdev descriptor is an error.

Hmm... that's an interesting discussion... what cases do you think it
would be fine to try to register twice an asd notifier?

Haven't write myself any piece of code using async framework, on a first
glance, trying to register twice sounds like an error to me.

Sakari, what do you think?

Thanks,
Mauro