Re: kern/48243 (inconsistant usage of 'up->parent' in usb_subr.c)

[dhol...@netbsd.org]

Synopsis: inconsistant usage of 'up->parent' in usb_subr.c

Responsible-Changed-From-To: gnats-admin->kern-bug-people
Responsible-Changed-By: dhol...@NetBSD.org
Responsible-Changed-When: Sat, 28 Sep 2013 07:18:02 +0000
Responsible-Changed-Why:
rescue this from the pending queue (came in with mistyped category)

[sk...@netbsd.org]

Synopsis: inconsistant usage of 'up->parent' in usb_subr.c

State-Changed-From-To: open->feedback
State-Changed-By: sk...@NetBSD.org
State-Changed-When: Mon, 03 Oct 2016 06:50:49 +0000
State-Changed-Why:
The only time up->up_parent is non-NULL is for a root hub. Here the
usbd_get_initial_ddsec can (should?) never fail and therefore the
usbd_reset_port won't happen.

Is the code as-is really a problem?

[Wolfgang Stukenbrock]

Hi, sorry for the delay, but I was offline for a while.

If a pointer can be NULL and not NULL then it always make sence to check this (at least in debug-builds) to catch
any situation where the expected state is not the expected one.
And if a call to a subroutine 'can (should?)' never fail, this should to be checked toö.

If you think the code is stable enought that it will never fail, you may add checks only for debug-builds to validate this.
This wouldn't harm performance in normal kernels.

This is my understanding from a 'stable' source that allows searching for errors if any happen.
And perhaps it would be a good idea to add a short comment when the pointer is NULL or not ..

As stated in the initial report, I found this by analysing a problem I've with something in the USB stack.
It is along time ago and I don't remember the cause why I've a look at it anymore - sorry.
As far as I remember, it was never the cause of a kernel crash - in that case I haven't classified the report as non-critical/medium.

--


Dr. Nagler & Company GmbH
Hauptstra_e 9
92253 Schnaittenbach

Tel. +49 9622/71 97-42
Fax +49 9622/71 97-50

Wolfgang.S...@nagler-company.com
http://www.nagler-company.com


Hauptsitz: Schnaittenbach
Handelregister: Amberg HRB
Gerichtsstand: Amberg
Steuernummer: 201/118/51825
USt.-ID-Nummer: DE 273143997
Gesch_ftsf_hrer: Dr. Martin Nagler

[Wolfgang Stukenbrock]

The following reply was made to PR kern/48243; it has been noted by GNATS.

From: Wolfgang Stukenbrock <Wolfgang.S...@nagler-company.com>
To: <gnats...@NetBSD.org>
Cc: <sk...@NetBSD.org>, <kern-bu...@netbsd.org>, <netbs...@netbsd.org>,
<gnats...@netbsd.org>, <Wolfgang.S...@nagler-company.com>
Subject: Re: kern/48243 (inconsistant usage of 'up->parent' in usb_subr.c)
Date: Fri, 14 Oct 2016 10:56:12 +0200

Hi, sorry for the delay, but I was offline for a while.

If a pointer can be NULL and not NULL then it always make sence to check th=

is (at least in debug-builds) to catch
any situation where the expected state is not the expected one.

And if a call to a subroutine 'can (should?)' never fail, this should to be=
checked to=F6.

If you think the code is stable enought that it will never fail, you may ad=

d checks only for debug-builds to validate this.
This wouldn't harm performance in normal kernels.

This is my understanding from a 'stable' source that allows searching for e=
rrors if any happen.
And perhaps it would be a good idea to add a short comment when the pointer=

is NULL or not ..

As stated in the initial report, I found this by analysing a problem I've w=

ith something in the USB stack.

It is along time ago and I don't remember the cause why I've a look at it a=
nymore - sorry.
As far as I remember, it was never the cause of a kernel crash - in that ca=

se I haven't classified the report as non-critical/medium.

On Mon, 3 Oct 2016 06:50:49 +0000
<sk...@NetBSD.org> wrote:

> Synopsis: inconsistant usage of 'up->parent' in usb_subr.c

>=20

> State-Changed-From-To: open->feedback
> State-Changed-By: sk...@NetBSD.org
> State-Changed-When: Mon, 03 Oct 2016 06:50:49 +0000
> State-Changed-Why:
> The only time up->up_parent is non-NULL is for a root hub. Here the
> usbd_get_initial_ddsec can (should?) never fail and therefore the
> usbd_reset_port won't happen.

>=20

> Is the code as-is really a problem?

>=20
>=20
>=20


--=20