linux-kernel - Re: [RFC PATCH 27/57] drivers: Unify the match prototype for bus_find_device with class_find

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] [day] [month] [year] [list]

Message-ID: <d51d8361-8807-5795-7da0-68c299217213@arm.com>
Date:   Tue, 4 Jun 2019 12:39:50 +0100
From:   Suzuki K Poulose <suzuki.poulose@....com>
To:     rafael@...nel.org
Cc:     linux-kernel@...r.kernel.org, gregkh@...uxfoundation.org,
        alexander.shishkin@...ux.intel.com, wsa@...-dreams.de,
        jic23@...nel.org, knaack.h@....de, grygorii.strashko@...com,
        davem@...emloft.net, bhelgaas@...gle.com, sebott@...ux.ibm.com,
        oberpar@...ux.ibm.com, freude@...ux.ibm.com, jejb@...ux.ibm.com,
        martin.petersen@...cle.com, andreas.noever@...il.com,
        michael.jamet@...el.com, balbi@...nel.org,
        david.kershner@...sys.com
Subject: Re: [RFC PATCH 27/57] drivers: Unify the match prototype for
 bus_find_device with class_find_device



On 04/06/2019 12:26, Rafael J. Wysocki wrote:
> On Mon, Jun 3, 2019 at 5:51 PM Suzuki K Poulose <suzuki.poulose@....com> wrote:
>>
>> We have iterators for devices by bus and class, with a supplied
>> "match" function to do the comparison. However, both of the helper
>> function have slightly different prototype for the "match" argument.
>>
>>   int (*) (struct device *dev, void *data)  // bus_find_device
>>    vs
>>   int (*) (struct device *dev, const void *data) // class_find_device
>>
>> Unify the prototype by promoting the match function to use that of
>> the class_find_device(). This will allow us to share the generic
>> match helpers with class_find_device() users.
> 
> The patch looks good to me, but the changelog might be a bit better.
> 
> It seems to be all about the bus_find_device() and class_find_device()
> prototype consolidation, so that the same pair of data and match()
> arguments can be passed to both of them, which then will allow some
> optimizations to be made, so what about the following:
> 
> "There is an arbitrary difference between the prototypes of
> bus_find_device() and class_find_device() preventing their callers
> from passing the same pair of data and match() arguments to both of
> them, which is the const qualifier used in the prototype of
> class_find_device().  If that qualifier is also used in the
> bus_find_device() prototype, it will be possible to pass the same
> match() callback function to both bus_find_device() and
> class_find_device(), which will allow some optimizations to be made in
> order to avoid code duplication going forward.
> 
> For this reason, change the prototype of bus_find_device() to match
> the prototype of class_find_device() and adjust its callers to use the
> const qualifier in accordance with the new prototype of it.".

Agreed, I will reword the description.

> 
> Also, it looks like there is no need to make all of the following
> changes in the series along with this one in one go and making them
> separately would be *much* better from the patch review perspective.

Sure. I started with the helpers in the hope that, I would need fewer
changes to individual subsystems, once I convert them to use the
new helpers.

i.e, driver A -> use new helper and the change the new helper.

rather than

change all callers of *_find_device() and then all to switch to new helper.

Anyways, looks like the latter is better in terms of splitting the series.
I will rework the series.

Thanks a lot for your input

Cheers
Suzuki