Re: [PATCH v2] usbip: tools: Fix read_usb_vudc_device() error path handling

[GwanYeong Kim <gy741.kim@gmail.com>]

On Wed, 16 Oct 2019 20:33:39 -0600
shuah <shuah@kernel.org> wrote:

> On 10/16/19 8:25 PM, GwanYeong Kim wrote:
> > cannot be less than 0 - fread() returns 0 on error.
> > 
> 
> This isn't really accurate right. fread() doesn't always
> return 0 in error. It could return < number of elements
> and set errno.
> 
> Please make changes to reflect that.

Will reflect this.

> 
> > Signed-off-by: GwanYeong Kim <gy741.kim@gmail.com>
> > ---
> >   tools/usb/usbip/libsrc/usbip_device_driver.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c
> > b/tools/usb/usbip/libsrc/usbip_device_driver.c index
> > 051d7d3f443b..959bb29d0477 100644 ---
> > a/tools/usb/usbip/libsrc/usbip_device_driver.c +++
> > b/tools/usb/usbip/libsrc/usbip_device_driver.c @@ -69,7 +69,7 @@
> > int read_usb_vudc_device(struct udev_device *sdev, struct
> > usbip_usb_device *dev) FILE *fd = NULL; struct udev_device *plat;
> >   	const char *speed;
> > -	int ret = 0;
> > +	size_t ret = 0;
> 
> You don't need to initialize this.

Exactly, because "ret" variable receives a "fread()" return value,

> 
> >   
> >   	plat = udev_device_get_parent(sdev);
> >   	path = udev_device_get_syspath(plat);
> > @@ -79,7 +79,7 @@ int read_usb_vudc_device(struct udev_device
> > *sdev, struct usbip_usb_device *dev) if (!fd)
> >   		return -1;
> >   	ret = fread((char *) &descr, sizeof(descr), 1, fd);
> > -	if (ret < 0)
> > +	if (ret != 1)
> 
> Why not print error message?


Sorry, I'll add a message.

How about this?

err("Cannot read vudc device descr file");


> 
> >   		goto err;
> >   	fclose(fd);
> >   
> > 
> 
> thanks,
> -- Shuah