Re: [bug report] pinctrl: add ZTE ZX pinctrl driver support

[Dan Carpenter <dan.carpenter@oracle.com>]

On Thu, Jun 29, 2017 at 10:30:55PM +0800, Shawn Guo wrote:
> Hi Dan,
> 
> On Thu, Jun 29, 2017 at 12:51:52PM +0300, Dan Carpenter wrote:
> > Hello Shawn Guo,
> > 
> > The patch cbff0c4d27f4: "pinctrl: add ZTE ZX pinctrl driver support"
> > from Apr 24, 2017, leads to the following static checker warning:
> > 
> > 	drivers/pinctrl/zte/pinctrl-zx.c:338 zx_pinctrl_build_state()
> > 	warn: passing devm_ allocated variable to kfree. 'functions'
> 
> I tried to see this warning with sparse, but failed.  Can you please
> tell how I can reproduced this static check warning?
> 

Hm.  It's a Smatch thing that I didn't push.  I'll push it in the next
day or two.

> > 
> > drivers/pinctrl/zte/pinctrl-zx.c
> >    293  
> >    294          /* Build function list from pin mux functions */
> >    295          functions = devm_kzalloc(&pdev->dev, info->npins * sizeof(*functions),
> >                 ^^^^^^^^^^^^^^^^^^^^^^^^
> > 
> >    296                                   GFP_KERNEL);
> >    297          if (!functions)
> >    298                  return -ENOMEM;
> >    299  
> >    300          nfunctions = 0;
> >    301          for (i = 0; i < info->npins; i++) {
> >    302                  const struct pinctrl_pin_desc *pindesc = info->pins + i;
> >    303                  struct zx_pin_data *data = pindesc->drv_data;
> >    304                  struct zx_mux_desc *mux;
> >    305  
> >    306                  /* Reserved pins do not have a drv_data at all */
> >    307                  if (!data)
> >    308                          continue;
> >    309  
> >    310                  /* Loop over all muxes for the pin */
> >    311                  mux = data->muxes;
> >    312                  while (mux->name) {
> >    313                          struct function_desc *func = functions;
> >    314  
> >    315                          /* Search function list for given mux */
> >    316                          while (func->name) {
> >    317                                  if (strcmp(mux->name, func->name) == 0) {
> >    318                                          /* Function exists */
> >    319                                          func->num_group_names++;
> >    320                                          break;
> >    321                                  }
> >    322                                  func++;
> >    323                          }
> >    324  
> >    325                          if (!func->name) {
> >    326                                  /* New function */
> >    327                                  func->name = mux->name;
> >    328                                  func->num_group_names = 1;
> >    329                                  radix_tree_insert(&pctldev->pin_function_tree,
> >    330                                                    nfunctions++, func);
> >    331                          }
> >    332  
> >    333                          mux++;
> >    334                  }
> >    335          }
> >    336  
> >    337          pctldev->num_functions = nfunctions;
> >    338          functions = krealloc(functions, nfunctions * sizeof(*functions),
> >                                      ^^^^^^^^^
> > Not allowed.
> 
> So I guess the best fix would be changing devm_kzalloc() above to
> kzalloc(), and managing the free procedure on my own, right?
>

Sounds good.

regards,
dan carpenter

> Shawn
> 
> > 
> >    339                               GFP_KERNEL);
> >    340  
> >    341          /* Find pin groups for every single function */
> >    342          for (i = 0; i < info->npins; i++) {