On Tue, Jun 29, 2021 at 07:17:20AM -0700, Matthias Kaehlcke wrote: > On Tue, Jun 29, 2021 at 09:55:58AM +0530, Manivannan Sadhasivam wrote: > > On Mon, Jun 28, 2021 at 04:03:14PM -0700, Matthias Kaehlcke wrote: > > > > [...] > > > > > > > > > > > A few more previous lines of code for context: > > > > > > int count = QMP_NUM_COOLING_RESOURCES; > > > > > > qmp->cooling_devs = devm_kcalloc(qmp->dev, count, > > > sizeof(*qmp->cooling_devs), > > > GFP_KERNEL); > > > > > > I would suggest to initialize 'count' to 0 from the start and pass > > > QMP_NUM_COOLING_RESOURCES to devm_kcalloc() rather than 'count', > > > instead of resetting 'count' afterwards. > > > > Yeah, I thought about it but the actual bug in the code is not resetting > > the count value to 0. So fixing this way seems a better option. > > I don't agree that it's the better option. IMO it's clearer to pass > the constant QMP_NUM_COOLING_RESOURCES directly to devm_kcalloc(), > rather than giving the impression that the number of allocated items > is variable. Repurposing variables can be confusing and led to this > bug. Also the resulting code doesn't need to re-initialize 'count'. I don't dis-agree with you on this :) Let me send v2 incorporating the comments. Thanks, Mani