Hi Biju, On Fri, Dec 7, 2018 at 10:34 AM Biju Das <biju.das@xxxxxxxxxxxxxx> wrote: > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc > > > > On 06/12/2018 15:49:57+0000, Biju Das wrote: > > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP > > > > pcf85263 rtc > > > > On Thu, Dec 6, 2018 at 4:24 PM Biju Das <biju.das@xxxxxxxxxxxxxx> > > wrote: > > > > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP > > > > > > pcf85263 rtc CC nvmem maintainer > > > > > > Given bytes should be 1, val should be a pointer to a single byte... > > > > > > What if bytes == 0? > > > > > > > > > > I doubt we get "bytes==0" because of the checks in " > > > > drivers/nvmem/core.c" > > > > > Function " bin_attr_nvmem_read/ bin_attr_nvmem_write". > > > > > > > > Depends. There are other functions calling nvmem_reg_{read,write}(), > > e.g. > > > > nvmem_device_{read,write}(). > > > > > > OK. In that case, I will return (-EINVAL) for "bytes !=1" > > > > I think it is probably better to ensure the nvmem core never passes an invalid > > number of bytes. All the ther RTC drivers make that assumption. > > In that case, I will do following checks in nvmem_device_{read,write}() before calling nvmem_reg_{read,write}(), > > nvmem_device_read > > /* Stop the user from reading */ > if (offset >= nvmem->size) > return 0; > > if (bytes == 0) > return -EINVAL; Why not 0? > > if (offset + bytes > nvmem->size) This might overflow, please use check_add_overflow(). > bytes = nvmem->size - offset; > > nvmem_device_write > > /* Stop the user from writing */ > if (offset >= nvmem->size) > return -EFBIG; ENOSPC? + same comments as for read. Oh, and offset is unsigned int instead of loff_t. Nobody's envisioning nvmem devices larger than 4 GiB? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds