lkml.org 
[lkml]   [2017]   [May]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 05/10] platform/x86: fujitsu-laptop: distinguish current uses of device-specific data
    On Mon, Apr 24, 2017 at 03:33:29PM +0200, Micha?? K??pie?? wrote:
    > In portions of the driver which use device-specific data, rename local
    > variables from fujitsu_bl and fujitsu_laptop to priv in order to clearly
    > distinguish these parts from code that uses module-wide data.
    >
    > Signed-off-by: Micha?? K??pie?? <kernel@kempniu.pl>
    > ---
    > drivers/platform/x86/fujitsu-laptop.c | 48 +++++++++++++++++------------------
    > 1 file changed, 24 insertions(+), 24 deletions(-)
    >
    > diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
    > index 5f6b34a97348..536b601c7067 100644
    > --- a/drivers/platform/x86/fujitsu-laptop.c
    > +++ b/drivers/platform/x86/fujitsu-laptop.c
    > @@ -369,26 +369,26 @@ static const struct key_entry keymap_backlight[] = {
    >
    > static int acpi_fujitsu_bl_input_setup(struct acpi_device *device)
    > {
    > - struct fujitsu_bl *fujitsu_bl = acpi_driver_data(device);
    > + struct fujitsu_bl *priv = acpi_driver_data(device);

    [cut]

    > static int fujitsu_backlight_register(struct acpi_device *device)
    > @@ -566,27 +566,27 @@ static const struct dmi_system_id fujitsu_laptop_dmi_table[] = {
    >
    > static int acpi_fujitsu_laptop_input_setup(struct acpi_device *device)
    > {
    > - struct fujitsu_laptop *fujitsu_laptop = acpi_driver_data(device);
    > + struct fujitsu_laptop *priv = acpi_driver_data(device);
    > int ret;

    Distinguishing between local and global use like this makes sense, but I
    feel we should stick with a slightly more descriptive name than "priv".
    Without any qualification, "priv" could refer to private device-specific
    data from either the fujitsu_bl or fujitsu_laptop drivers. From the source
    it is far from obvious which is being accessed in a given function. If we
    implemented only a single ACPI device driver then this would be largely a
    moot point, but as there are two within the one module the loss of the
    description could make it harder to follow the code later on.

    Could we use "bl_priv" and "laptop_priv" for example, so as to provide a
    clue within the source code as to what exactly is being referenced?
    Obviously it doesn't provide any compile time type checking, but it's better
    than nothing.

    Regards
    jonathan

    \
     
     \ /
      Last update: 2017-05-01 15:42    [W:4.693 / U:0.424 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site