Re: [PATCH v2] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
Hi YueHaibing, Thank you for the patch! Yet something to improve: [auto build test ERROR on pinctrl/devel] [also build test ERROR on v4.18-rc7 next-20180731] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/YueHaibing/pinctrl-berlin-fix-pctrl-functions-allocation-in-berlin_pinctrl_build_state/20180801-131813 base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel config: xtensa-allyesconfig (attached as .config) compiler: xtensa-linux-gcc (GCC) 8.1.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=8.1.0 make.cross ARCH=xtensa All errors (new ones prefixed by >>): drivers/pinctrl/berlin/berlin.c: In function 'berlin_pinctrl_build_state': >> drivers/pinctrl/berlin/berlin.c:259:5: error: too few arguments to function >> 'kfree' kfree( ); ^ In file included from drivers/pinctrl/berlin/berlin.c:20: include/linux/slab.h:183:6: note: declared here void kfree(const void *); ^ vim +/kfree +259 drivers/pinctrl/berlin/berlin.c 202 203 static int berlin_pinctrl_build_state(struct platform_device *pdev) 204 { 205 struct berlin_pinctrl *pctrl = platform_get_drvdata(pdev); 206 const struct berlin_desc_group *desc_group; 207 const struct berlin_desc_function *desc_function; 208 int i, max_functions = 0; 209 210 pctrl->nfunctions = 0; 211 212 for (i = 0; i < pctrl->desc->ngroups; i++) { 213 desc_group = pctrl->desc->groups + i; 214 /* compute the maxiumum number of functions a group can have */ 215 max_functions += 1 << (desc_group->bit_width + 1); 216 } 217 218 /* we will reallocate later */ 219 pctrl->functions = kcalloc(max_functions, 220 sizeof(*pctrl->functions), GFP_KERNEL); 221 if (!pctrl->functions) 222 return -ENOMEM; 223 224 /* register all functions */ 225 for (i = 0; i < pctrl->desc->ngroups; i++) { 226 desc_group = pctrl->desc->groups + i; 227 desc_function = desc_group->functions; 228 229 while (desc_function->name) { 230 berlin_pinctrl_add_function(pctrl, desc_function->name); 231 desc_function++; 232 } 233 } 234 235 pctrl->functions = krealloc(pctrl->functions, 236 pctrl->nfunctions * sizeof(*pctrl->functions), 237 GFP_KERNEL); 238 239 /* map functions to theirs groups */ 240 for (i = 0; i < pctrl->desc->ngroups; i++) { 241 desc_group = pctrl->desc->groups + i; 242 desc_function = desc_group->functions; 243 244 while (desc_function->name) { 245 struct berlin_pinctrl_function 246 *function = pctrl->functions; 247 const char **groups; 248 bool found = false; 249 250 while (function->name) { 251 if (!strcmp(desc_function->name, function->name)) { 252 found = true; 253 break; 254 } 255 function++; 256 } 257 258 if (!found) { > 259 kfree( ); 260 return -EINVAL; 261 } 262 263 if (!function->groups) { 264 function->groups = 265 devm_kcalloc(>dev, 266 function->ngroups, 267 sizeof(char *), 268 GFP_KERNEL); 269 270 if (!function->groups) { 271 kfree(pctrl->functions); 272 return -ENOMEM; 273 } 274 } 275 276 groups = function->groups; 277 while (*groups) 278
Re: [PATCH v2] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
Hi YueHaibing, Thank you for the patch! Yet something to improve: [auto build test ERROR on pinctrl/devel] [also build test ERROR on v4.18-rc7 next-20180731] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/YueHaibing/pinctrl-berlin-fix-pctrl-functions-allocation-in-berlin_pinctrl_build_state/20180801-131813 base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel config: xtensa-allyesconfig (attached as .config) compiler: xtensa-linux-gcc (GCC) 8.1.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=8.1.0 make.cross ARCH=xtensa All errors (new ones prefixed by >>): drivers/pinctrl/berlin/berlin.c: In function 'berlin_pinctrl_build_state': >> drivers/pinctrl/berlin/berlin.c:259:5: error: too few arguments to function >> 'kfree' kfree( ); ^ In file included from drivers/pinctrl/berlin/berlin.c:20: include/linux/slab.h:183:6: note: declared here void kfree(const void *); ^ vim +/kfree +259 drivers/pinctrl/berlin/berlin.c 202 203 static int berlin_pinctrl_build_state(struct platform_device *pdev) 204 { 205 struct berlin_pinctrl *pctrl = platform_get_drvdata(pdev); 206 const struct berlin_desc_group *desc_group; 207 const struct berlin_desc_function *desc_function; 208 int i, max_functions = 0; 209 210 pctrl->nfunctions = 0; 211 212 for (i = 0; i < pctrl->desc->ngroups; i++) { 213 desc_group = pctrl->desc->groups + i; 214 /* compute the maxiumum number of functions a group can have */ 215 max_functions += 1 << (desc_group->bit_width + 1); 216 } 217 218 /* we will reallocate later */ 219 pctrl->functions = kcalloc(max_functions, 220 sizeof(*pctrl->functions), GFP_KERNEL); 221 if (!pctrl->functions) 222 return -ENOMEM; 223 224 /* register all functions */ 225 for (i = 0; i < pctrl->desc->ngroups; i++) { 226 desc_group = pctrl->desc->groups + i; 227 desc_function = desc_group->functions; 228 229 while (desc_function->name) { 230 berlin_pinctrl_add_function(pctrl, desc_function->name); 231 desc_function++; 232 } 233 } 234 235 pctrl->functions = krealloc(pctrl->functions, 236 pctrl->nfunctions * sizeof(*pctrl->functions), 237 GFP_KERNEL); 238 239 /* map functions to theirs groups */ 240 for (i = 0; i < pctrl->desc->ngroups; i++) { 241 desc_group = pctrl->desc->groups + i; 242 desc_function = desc_group->functions; 243 244 while (desc_function->name) { 245 struct berlin_pinctrl_function 246 *function = pctrl->functions; 247 const char **groups; 248 bool found = false; 249 250 while (function->name) { 251 if (!strcmp(desc_function->name, function->name)) { 252 found = true; 253 break; 254 } 255 function++; 256 } 257 258 if (!found) { > 259 kfree( ); 260 return -EINVAL; 261 } 262 263 if (!function->groups) { 264 function->groups = 265 devm_kcalloc(>dev, 266 function->ngroups, 267 sizeof(char *), 268 GFP_KERNEL); 269 270 if (!function->groups) { 271 kfree(pctrl->functions); 272 return -ENOMEM; 273 } 274 } 275 276 groups = function->groups; 277 while (*groups) 278
Re: [PATCH v2] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
Sorry, I send a wrong patch, pls ignore this. On 2018/8/1 13:02, YueHaibing wrote: > fixes following Smatch static check warning: > > drivers/pinctrl/berlin/berlin.c:237 berlin_pinctrl_build_state() > warn: passing devm_ allocated variable to kfree. 'pctrl->functions' > > As we will be calling krealloc() on pointer 'pctrl->functions', which means > kfree() will be called in there, devm_kzalloc() shouldn't be used with > the allocation in the first place. Fix the warning by calling kcalloc() > and managing the free procedure in error path on our own. > > Fixes: 3de68d331c24 ("pinctrl: berlin: add the core pinctrl driver for > Marvell Berlin SoCs") > Signed-off-by: YueHaibing > --- > v2: free pctrl->functions instead of function as Jisheng Zhang suggested > --- > drivers/pinctrl/berlin/berlin.c | 14 -- > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/pinctrl/berlin/berlin.c b/drivers/pinctrl/berlin/berlin.c > index d6d183e..b5903ff 100644 > --- a/drivers/pinctrl/berlin/berlin.c > +++ b/drivers/pinctrl/berlin/berlin.c > @@ -216,10 +216,8 @@ static int berlin_pinctrl_build_state(struct > platform_device *pdev) > } > > /* we will reallocate later */ > - pctrl->functions = devm_kcalloc(>dev, > - max_functions, > - sizeof(*pctrl->functions), > - GFP_KERNEL); > + pctrl->functions = kcalloc(max_functions, > +sizeof(*pctrl->functions), GFP_KERNEL); > if (!pctrl->functions) > return -ENOMEM; > > @@ -257,8 +255,10 @@ static int berlin_pinctrl_build_state(struct > platform_device *pdev) > function++; > } > > - if (!found) > + if (!found) { > + kfree( ); > return -EINVAL; > + } > > if (!function->groups) { > function->groups = > @@ -267,8 +267,10 @@ static int berlin_pinctrl_build_state(struct > platform_device *pdev) >sizeof(char *), >GFP_KERNEL); > > - if (!function->groups) > + if (!function->groups) { > + kfree(pctrl->functions); > return -ENOMEM; > + } > } > > groups = function->groups; >
Re: [PATCH v2] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
Sorry, I send a wrong patch, pls ignore this. On 2018/8/1 13:02, YueHaibing wrote: > fixes following Smatch static check warning: > > drivers/pinctrl/berlin/berlin.c:237 berlin_pinctrl_build_state() > warn: passing devm_ allocated variable to kfree. 'pctrl->functions' > > As we will be calling krealloc() on pointer 'pctrl->functions', which means > kfree() will be called in there, devm_kzalloc() shouldn't be used with > the allocation in the first place. Fix the warning by calling kcalloc() > and managing the free procedure in error path on our own. > > Fixes: 3de68d331c24 ("pinctrl: berlin: add the core pinctrl driver for > Marvell Berlin SoCs") > Signed-off-by: YueHaibing > --- > v2: free pctrl->functions instead of function as Jisheng Zhang suggested > --- > drivers/pinctrl/berlin/berlin.c | 14 -- > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/pinctrl/berlin/berlin.c b/drivers/pinctrl/berlin/berlin.c > index d6d183e..b5903ff 100644 > --- a/drivers/pinctrl/berlin/berlin.c > +++ b/drivers/pinctrl/berlin/berlin.c > @@ -216,10 +216,8 @@ static int berlin_pinctrl_build_state(struct > platform_device *pdev) > } > > /* we will reallocate later */ > - pctrl->functions = devm_kcalloc(>dev, > - max_functions, > - sizeof(*pctrl->functions), > - GFP_KERNEL); > + pctrl->functions = kcalloc(max_functions, > +sizeof(*pctrl->functions), GFP_KERNEL); > if (!pctrl->functions) > return -ENOMEM; > > @@ -257,8 +255,10 @@ static int berlin_pinctrl_build_state(struct > platform_device *pdev) > function++; > } > > - if (!found) > + if (!found) { > + kfree( ); > return -EINVAL; > + } > > if (!function->groups) { > function->groups = > @@ -267,8 +267,10 @@ static int berlin_pinctrl_build_state(struct > platform_device *pdev) >sizeof(char *), >GFP_KERNEL); > > - if (!function->groups) > + if (!function->groups) { > + kfree(pctrl->functions); > return -ENOMEM; > + } > } > > groups = function->groups; >
[PATCH v2] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
fixes following Smatch static check warning: drivers/pinctrl/berlin/berlin.c:237 berlin_pinctrl_build_state() warn: passing devm_ allocated variable to kfree. 'pctrl->functions' As we will be calling krealloc() on pointer 'pctrl->functions', which means kfree() will be called in there, devm_kzalloc() shouldn't be used with the allocation in the first place. Fix the warning by calling kcalloc() and managing the free procedure in error path on our own. Fixes: 3de68d331c24 ("pinctrl: berlin: add the core pinctrl driver for Marvell Berlin SoCs") Signed-off-by: YueHaibing --- v2: free pctrl->functions instead of function as Jisheng Zhang suggested --- drivers/pinctrl/berlin/berlin.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/pinctrl/berlin/berlin.c b/drivers/pinctrl/berlin/berlin.c index d6d183e..b5903ff 100644 --- a/drivers/pinctrl/berlin/berlin.c +++ b/drivers/pinctrl/berlin/berlin.c @@ -216,10 +216,8 @@ static int berlin_pinctrl_build_state(struct platform_device *pdev) } /* we will reallocate later */ - pctrl->functions = devm_kcalloc(>dev, - max_functions, - sizeof(*pctrl->functions), - GFP_KERNEL); + pctrl->functions = kcalloc(max_functions, + sizeof(*pctrl->functions), GFP_KERNEL); if (!pctrl->functions) return -ENOMEM; @@ -257,8 +255,10 @@ static int berlin_pinctrl_build_state(struct platform_device *pdev) function++; } - if (!found) + if (!found) { + kfree( ); return -EINVAL; + } if (!function->groups) { function->groups = @@ -267,8 +267,10 @@ static int berlin_pinctrl_build_state(struct platform_device *pdev) sizeof(char *), GFP_KERNEL); - if (!function->groups) + if (!function->groups) { + kfree(pctrl->functions); return -ENOMEM; + } } groups = function->groups; -- 2.7.0
[PATCH v2] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
fixes following Smatch static check warning: drivers/pinctrl/berlin/berlin.c:237 berlin_pinctrl_build_state() warn: passing devm_ allocated variable to kfree. 'pctrl->functions' As we will be calling krealloc() on pointer 'pctrl->functions', which means kfree() will be called in there, devm_kzalloc() shouldn't be used with the allocation in the first place. Fix the warning by calling kcalloc() and managing the free procedure in error path on our own. Fixes: 3de68d331c24 ("pinctrl: berlin: add the core pinctrl driver for Marvell Berlin SoCs") Signed-off-by: YueHaibing --- v2: free pctrl->functions instead of function as Jisheng Zhang suggested --- drivers/pinctrl/berlin/berlin.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/pinctrl/berlin/berlin.c b/drivers/pinctrl/berlin/berlin.c index d6d183e..b5903ff 100644 --- a/drivers/pinctrl/berlin/berlin.c +++ b/drivers/pinctrl/berlin/berlin.c @@ -216,10 +216,8 @@ static int berlin_pinctrl_build_state(struct platform_device *pdev) } /* we will reallocate later */ - pctrl->functions = devm_kcalloc(>dev, - max_functions, - sizeof(*pctrl->functions), - GFP_KERNEL); + pctrl->functions = kcalloc(max_functions, + sizeof(*pctrl->functions), GFP_KERNEL); if (!pctrl->functions) return -ENOMEM; @@ -257,8 +255,10 @@ static int berlin_pinctrl_build_state(struct platform_device *pdev) function++; } - if (!found) + if (!found) { + kfree( ); return -EINVAL; + } if (!function->groups) { function->groups = @@ -267,8 +267,10 @@ static int berlin_pinctrl_build_state(struct platform_device *pdev) sizeof(char *), GFP_KERNEL); - if (!function->groups) + if (!function->groups) { + kfree(pctrl->functions); return -ENOMEM; + } } groups = function->groups; -- 2.7.0