linux-kernel - Re: [PATCH 4/9] pinctrl: sunxi: Deal with configless pins

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]

Date:   Fri, 7 Oct 2016 11:02:53 +0200
From:   Maxime Ripard <maxime.ripard@...e-electrons.com>
To:     Chen-Yu Tsai <wens@...e.org>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/9] pinctrl: sunxi: Deal with configless pins

Hi,

On Tue, Oct 04, 2016 at 10:28:18AM +0800, Chen-Yu Tsai wrote:
> >         if (sunxi_pctrl_has_drive_prop(node)) {
> >                 int drive = sunxi_pctrl_parse_drive_prop(node);
> > -               if (drive < 0)
> > +               if (drive < 0) {
> > +                       ret = -EINVAL;
> 
> Why not just pass the error code returned from the parse function?

Yep, I'll change that.

> > -               (*map)[i].type = PIN_MAP_TYPE_CONFIGS_GROUP;
> > -               (*map)[i].data.configs.group_or_pin = group;
> > -               (*map)[i].data.configs.configs = pinconfig;
> > -               (*map)[i].data.configs.num_configs = configlen;
> > -
> > -               i++;
> > +               if (pinconfig) {
> > +                       (*map)[i].type = PIN_MAP_TYPE_CONFIGS_GROUP;
> > +                       (*map)[i].data.configs.group_or_pin = group;
> > +                       (*map)[i].data.configs.configs = pinconfig;
> > +                       (*map)[i].data.configs.num_configs = configlen;
> > +                       i++;
> > +               }
> >         }
> >
> > -       *num_maps = nmaps;
> > +       *num_maps = i;
> 
> Thought: should we do a krealloc to shrink the array?

Yep, I'll make an additional patch to fix that.

> 
> >
> >         return 0;
> >
> > @@ -342,8 +357,13 @@ static void sunxi_pctrl_dt_free_map(struct pinctrl_dev *pctldev,
> >                                     struct pinctrl_map *map,
> >                                     unsigned num_maps)
> >  {
> > +       unsigned long *pinconfig;
> > +
> >         /* All the maps have the same pin config, free only the first one */
> > -       kfree(map[0].data.configs.configs);
> > +       pinconfig = map[0].data.configs.configs;
> > +       if (pinconfig)
> > +               kfree(pinconfig);
> 
> Passing NULL to kfree is allowed. (It becomes a no-op.)
> So you could leave this function alone.

And I'll change that as well.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)