linux-kernel - Re: [PATCH v4 4/4] base/platform: Remove code duplication

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: <20150607143220.B3807C42468@trevor.secretlab.ca>
Date:	Sun, 07 Jun 2015 15:32:20 +0100
From:	Grant Likely <grant.likely@...aro.org>
To:	Rob Herring <robherring2@...il.com>,
	Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Rob Herring <robh+dt@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Jakub Sitnicki <jsitnicki@...il.com>,
	Vivek Goyal <vgoyal@...hat.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Jiang Liu <jiang.liu@...ux.intel.com>,
	Mike Travis <travis@....com>,
	Thierry Reding <treding@...dia.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH v4 4/4] base/platform: Remove code duplication

On Wed, 13 May 2015 09:03:31 -0500
, Rob Herring <robherring2@...il.com>
 wrote:
> On Thu, Apr 23, 2015 at 8:58 AM, Ricardo Ribalda Delgado
> <ricardo.ribalda@...il.com> wrote:
> > Failure path of platform_device_add was almost the same as
> > platform_device_del. Refactor same code in a function.
> >
> > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>
> 
> Clean-ups should come first in the series. Otherwise:
> 
> Acked-by: Rob Herring <robh@...nel.org>

I disagree in this case, but only because the bug fix needs to be fixed
immediately, but the refactoring can wait for v4.1. Ricardo, if you can
confirm my patch solves the problem, then I'll push it to Linus, and you
can respin this patch to match.

g.

> 
> > ---
> >  drivers/base/platform.c | 60 +++++++++++++++++++++----------------------------
> >  1 file changed, 25 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > index 2e7e904..152d84d 100644
> > --- a/drivers/base/platform.c
> > +++ b/drivers/base/platform.c
> > @@ -298,6 +298,25 @@ int platform_device_add_data(struct platform_device *pdev, const void *data,
> >  }
> >  EXPORT_SYMBOL_GPL(platform_device_add_data);
> >
> > +static void platform_device_cleanout(struct platform_device *pdev, int n_res)
> > +{
> > +       int i;
> > +
> > +       if (pdev->id_auto) {
> > +               ida_simple_remove(&platform_devid_ida, pdev->id);
> > +               pdev->id = PLATFORM_DEVID_AUTO;
> > +       }
> > +
> > +       for (i = 0; i < n_res; i++) {
> > +               struct resource *r = &pdev->resource[i];
> > +               unsigned long type = resource_type(r);
> > +
> > +               if ((type == IORESOURCE_MEM || type == IORESOURCE_IO) &&
> > +                               r->parent)
> > +                       release_resource(r);
> > +       }
> > +}
> > +
> >  /**
> >   * platform_device_add - add a platform device to device hierarchy
> >   * @pdev: platform device we're adding
> > @@ -371,23 +390,8 @@ int platform_device_add(struct platform_device *pdev)
> >                  dev_name(&pdev->dev), dev_name(pdev->dev.parent));
> >
> >         ret = device_add(&pdev->dev);
> > -       if (ret == 0)
> > -               return ret;
> > -
> > -       /* Failure path */
> > -       if (pdev->id_auto) {
> > -               ida_simple_remove(&platform_devid_ida, pdev->id);
> > -               pdev->id = PLATFORM_DEVID_AUTO;
> > -       }
> > -
> > -       while (--i >= 0) {
> > -               struct resource *r = &pdev->resource[i];
> > -               unsigned long type = resource_type(r);
> > -
> > -               if ((type == IORESOURCE_MEM || type == IORESOURCE_IO) &&
> > -                               r->parent)
> > -                       release_resource(r);
> > -       }
> > +       if (ret)
> > +               platform_device_cleanout(pdev, i);
> >
> >         return ret;
> >  }
> > @@ -403,25 +407,11 @@ EXPORT_SYMBOL_GPL(platform_device_add);
> >   */
> >  void platform_device_del(struct platform_device *pdev)
> >  {
> > -       int i;
> > -
> > -       if (pdev) {
> > -               device_del(&pdev->dev);
> > -
> > -               if (pdev->id_auto) {
> > -                       ida_simple_remove(&platform_devid_ida, pdev->id);
> > -                       pdev->id = PLATFORM_DEVID_AUTO;
> > -               }
> > -
> > -               for (i = 0; i < pdev->num_resources; i++) {
> > -                       struct resource *r = &pdev->resource[i];
> > -                       unsigned long type = resource_type(r);
> > +       if (!pdev)
> > +               return;
> >
> > -                       if ((type == IORESOURCE_MEM || type == IORESOURCE_IO) &&
> > -                                       r->parent)
> > -                               release_resource(r);
> > -               }
> > -       }
> > +       device_del(&pdev->dev);
> > +       platform_device_cleanout(pdev, pdev->num_resources);
> >  }
> >  EXPORT_SYMBOL_GPL(platform_device_del);
> >
> > --
> > 2.1.4
> >

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/