Re: [PATCH v2 3/3] ACPI/device_sysfs: Clean up checkpatch errors

[Rafael J. Wysocki]

On Friday, April 29, 2016 09:13:28 PM Dall, Betty wrote:
> On 04/29/2016 02:19 PM, Rafael J. Wysocki wrote:
> > On Fri, Apr 29, 2016 at 9:21 PM, Betty Dall <betty.dall@xxxxxxx> wrote:
> >> Cleaning up five existing checkpatch errors in device_sysfs.c since the
> >> file is being changed.
> >>
> >> Signed-off-by: Betty Dall <betty.dall@xxxxxxx>
> >> ---
> >>  drivers/acpi/device_sysfs.c | 22 ++++++++++++++--------
> >>  1 file changed, 14 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
> >> index e556a3e..5aaebec 100644
> >> --- a/drivers/acpi/device_sysfs.c
> >> +++ b/drivers/acpi/device_sysfs.c
> >> @@ -35,7 +35,7 @@ static ssize_t acpi_object_path(acpi_handle handle, char *buf)
> >>         if (result)
> >>                 return result;
> >>
> >> -       result = sprintf(buf, "%s\n", (char*)path.pointer);
> >> +       result = sprintf(buf, "%s\n", (char *)path.pointer);
> > 
> > OK
> > 
> >>         kfree(path.pointer);
> >>         return result;
> >>  }
> >> @@ -81,6 +81,7 @@ static const struct sysfs_ops acpi_data_node_sysfs_ops = {
> >>  static void acpi_data_node_release(struct kobject *kobj)
> >>  {
> >>         struct acpi_data_node *dn = to_data_node(kobj);
> >> +
> > 
> > Maybe.
> 
> Checkpatch wants a blank line after declarations.

But sometimes they are not really useful.  As in this case IMO.

> >>         complete(&dn->kobj_done);
> >>  }
> >>
> >> @@ -106,7 +107,8 @@ static void acpi_expose_nondev_subnodes(struct kobject *kobj,
> >>                 ret = kobject_init_and_add(&dn->kobj, &acpi_data_node_ktype,
> >>                                            kobj, "%s", dn->name);
> >>                 if (ret)
> >> -                       acpi_handle_err(dn->handle, "Failed to expose (%d)\n", ret);
> >> +                       acpi_handle_err(dn->handle,
> >> +                               "Failed to expose (%d)\n", ret);
> > 
> > No.  checkpatch is wrong here.
> 
> Ok - that was just an 80 char warning.
> 
> >>                 else
> >>                         acpi_expose_nondev_subnodes(&dn->kobj, &dn->data);
> >>         }
> >> @@ -333,7 +335,9 @@ int acpi_device_modalias(struct device *dev, char *buf, int size)
> >>  EXPORT_SYMBOL_GPL(acpi_device_modalias);
> >>
> >>  static ssize_t
> >> -acpi_device_modalias_show(struct device *dev, struct device_attribute *attr, char *buf) {
> >> +acpi_device_modalias_show(struct device *dev,
> >> +                       struct device_attribute *attr, char *buf)
> > 
> > The brace should go to the new line, but it's better if the header
> > takes one line only.
> 
> Ok - I was trying to clean up the 80 character warning, but I see your
> point.
> 
> >> +{
> >>         return __acpi_device_modalias(to_acpi_device(dev), buf, 1024);
> >>  }
> >>  static DEVICE_ATTR(modalias, 0444, acpi_device_modalias_show, NULL);
> >> @@ -397,7 +401,9 @@ acpi_eject_store(struct device *d, struct device_attribute *attr,
> >>  static DEVICE_ATTR(eject, 0200, NULL, acpi_eject_store);
> >>
> >>  static ssize_t
> >> -acpi_device_hid_show(struct device *dev, struct device_attribute *attr, char *buf) {
> >> +acpi_device_hid_show(struct device *dev,
> >> +       struct device_attribute *attr, char *buf)
> > 
> > Ditto.
> 
> OK.
> 
> >> +{
> >>         struct acpi_device *acpi_dev = to_acpi_device(dev);
> >>
> >>         return sprintf(buf, "%s\n", acpi_device_hid(acpi_dev));
> >> @@ -568,10 +574,10 @@ int acpi_device_setup_files(struct acpi_device *dev)
> >>                         goto end;
> >>         }
> >>
> >> -        /*
> >> -         * If device has _EJ0, 'eject' file is created that is used to trigger
> >> -         * hot-removal function from userland.
> >> -         */
> >> +       /*
> >> +        * If device has _EJ0, 'eject' file is created that is used to trigger
> >> +        * hot-removal function from userland.
> >> +        */
> > 
> > What's the problem with this comment?
> 
> They were spaces - not a tab.

Ah, whitespace damage.  OK