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

From: GwanYeong Kim <gy741.kim@gmail.com>
To: shuah <shuah@kernel.org>
Cc: valentina.manea.m@gmail.com, gregkh@linuxfoundation.org,
	allison@lohutok.net, opensource@jilayne.com,
	changcheng.liu@intel.com, tglx@linutronix.de,
	linux-usb@vger.kernel.org
Subject: Re: [PATCH] usbip: tools: Fix read_usb_vudc_device() error path handling
Date: Wed, 16 Oct 2019 13:38:25 +0900	[thread overview]
Message-ID: <20191016133825.00006e29@gmail.com> (raw)
In-Reply-To: <bb2bef23-336a-2f28-8faa-00e4adbb76d9@kernel.org>

On Tue, 15 Oct 2019 17:14:32 -0600
shuah <shuah@kernel.org> wrote:

> On 10/15/19 7:14 AM, GwanYeong Kim wrote:
> > cannot be less than 0 - fread() returns 0 on error.
> > 
> > Signed-off-by: GwanYeong Kim <gy741.kim@gmail.com>
> > ---
> >   tools/usb/usbip/libsrc/usbip_device_driver.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c
> > b/tools/usb/usbip/libsrc/usbip_device_driver.c index
> > 051d7d3f443b..49760b98aabc 100644 ---
> > a/tools/usb/usbip/libsrc/usbip_device_driver.c +++
> > b/tools/usb/usbip/libsrc/usbip_device_driver.c @@ -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 != sizeof(descr))
> 
> Are you sure this check is correct? fread() returns the number
> of elements read, # elements = 1  in this case.

Thank you for pointing this out.
Sorry for my mistake.

> fread() returns 0 when size or # of elements is 0 and in other
> error cases it will return < # of elements. I would think you
> want to check ret != 1 here.

You're right.
This should be changed to "ret != 1".

Should I send a new patch?

Regards,
GwanYeong Kim