On 8/27/21 11:14 AM, David Laight wrote:
From: Dan Carpenter
Sent: 26 August 2021 12:21
...
> > > ...
> > > > + len += snprintf(page + len, count - len,
> > > > "rtw_read8(0x%x)=0x%x\n",
> > > > + proc_get_read_addr, (u8) tmp);
> > >
> > > That is broken.
> > >
> >
> > Don't get it, sorry. Previous code did exactly the same thing, but
> > didn't check if read() was successful.
>
> Look up the return value of snprintf().
>
It's hard to understand what you are saying. I think you are confusing
libc snprintf with the kernel snprintf? In libc the snprintf function
can return negatives but in the kernel it cannot. This is not going
to change. Any code which checks for negative snprintf returns in the
kernel is wrong and should be fixed.
Anyway, the code works fine. snprintf here is going to return a number
between 18-32 range. It's not going to overflow the PAGE_SIZE buffer.
IIRC it is also in a loop ...
Maybe, but the idiom is just broken.
Largely the result of snprintf() is never the value you are looking
for and should be ignored.
AFAIK, snprintf return value is largely used in sysfs at least.
$ grep -Ir "= snprintf" | wc -l
1200
Anyway, I will convert this dead code to sysfs interface and maybe
snprintf will go away...
Thank you for your help and comments
Userspace fprintf() is even worse.
If you care about the write failing you need to call fflush()
and then ferror() (typically before fclose()).
Fortunately I've never seen a 'must check' attribute on it.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
With regards,
Pavel Skripkin
