Re: [PATCH] usb: core: Strengthen error handling in hub_hub_status()

[Alan Stern]

On Tue, Jul 14, 2026 at 07:48:14PM +0200, Oliver Neukum wrote:
> On 14.07.26 18:32, Alan Stern wrote:
> 
> > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > > index 5262e11c12cd..0d2166b8923a 100644
> > > --- a/drivers/usb/core/hub.c
> > > +++ b/drivers/usb/core/hub.c
> > > @@ -991,10 +991,12 @@ static int hub_hub_status(struct usb_hub *hub,
> > >   	mutex_lock(&hub->status_mutex);
> > >   	ret = get_hub_status(hub->hdev, &hub->status->hub);
> > > -	if (ret < 0) {
> > > +	if (ret < sizeof(hub->status->hub)) {
> > >   		if (ret != -ENODEV)
> > >   			dev_err(hub->intfdev,
> > >   				"%s failed (err = %d)\n", __func__, ret);
> > > +		if (ret >= 0)
> > > +			ret = -EIO;
> > 
> > It would be better to put these two lines above the preceding test.
> > That way it won't print confusing things like "hub_hub_status failed
> > (err = 2)".
> I am afraid I have to disagree. We would in effect say that get_hub_status()
> has returned -EIO. That is simply not true.

On the contrary: The message would say that hub_hub_status is returning 
-EIO, which _is_ true.

Of course, the error message could instead be changed in the way you 
suggest:

				dev_err(hub->intfev,
					"get_hub_status failed (%d)\n", ret);

Either my change or yours would be okay with me, but I don't like what 
the patch does.

(Another question is whether this message deserves to be dev_err().  
There isn't anything the user can do about it when it happens, so maybe 
it should be dev_dbg() instead.)

Alan Stern