On Wed, 2023-08-23 at 19:26 +0300, Andy Shevchenko wrote: > On Wed, Aug 23, 2023 at 05:58:06PM +0200, Angel Iglesias wrote: > > Improve device detection in certain chip families known to have various > > chip ids. > > IDs > > ... > > > #include <linux/completion.h> > > #include <linux/pm_runtime.h> > > #include <linux/random.h> > > +#include <linux/overflow.h> > > Please, preserve ordering. > > ... > > > struct bmp280_data *data; > > struct gpio_desc *gpiod; > > unsigned int chip_id; > > - int ret; > > + int ret, i; > > unsigned int i; > > ... > > > + if (i == data->chip_info->num_chip_id) { > > + size_t nbuf; > > + char *buf; > > + > > + // 0x<id>, so four chars per number plus one space + ENDL > > > + if (check_mul_overflow(5, data->chip_info->num_chip_id, > > &nbuf)) > > + return ret; > > + > > + buf = kmalloc_array(nbuf, sizeof(char), GFP_KERNEL); > > We almost never do a array allocation for byte sizes. Instead of the above you > need to use > > buf = kmalloc_array(data->chip_info->num_chip_id, 5, > GFP_KERNEL); > > > + if (!buf) > > This check assumes that num_chip_id is never 0, so... > > > + return ret; > > + > > + for (i = 0; i < data->chip_info->num_chip_id; i++) > > + snprintf(&buf[i*5], nbuf, "0x%x ", data->chip_info- > > >chip_id[i]); > > > + buf[nbuf-1] = '\0'; > > ...this is redundant assignment. sprintf() guarantees the NUL termination. > > > + > > + dev_err(dev, "bad chip id: expected [ %s ] got 0x%x\n", buf, > > chip_id); > > "...expected one of..." > > > + kfree(buf); > > + return ret; > > Oh, I didn't get that you allocated memory only to print a message... Yes, I just wanted to print the known IDs like before in a readable manner. If it is overkill, I'll change it for a more generic error message. > > > } > Kind regards, Angel