re: CVS commit: src/sys/dev/usb

[matthew green <mrg%eterna23.net@localhost>]

"Hans Rosenfeld" writes:
> Module Name:	src
> Committed By:	hans
> Date:		Sun Mar 23 12:08:13 UTC 2025
>
> Modified Files:
> 	src/sys/dev/usb: ums.c
>
> Log Message:
> ums(4): make sure the device is enabled before calling uhidev_close()
>
> Same issue as in uts(4), his check was already there, but only enabled
> for DIAGNOSTIC kernels.  The check and early return are always needed,
> but the message should only be printed in DIAGNOSTIC kernels.

hm.  does this really change anything?

---
Static void
ums_disable(void *v)
{
        struct ums_softc *sc = v;

        UMSHIST_FUNC(); UMSHIST_CALLARGS("sc=%jx\n", (uintptr_t)sc, 0, 0, 0);

        if (!sc->sc_enabled) {
#ifdef DIAGNOSTIC
                printf("ums_disable: not enabled\n");
#endif
                return;
        }

        if (sc->sc_enabled) {
                sc->sc_enabled = 0;
                if (!sc->sc_alwayson)
                        uhidev_close(sc->sc_hdev);
        }
}
---

whether the DIAGNOSTIC check covers the return or not, in the case
it isn't enabled, it should end up doing nothing anyway?  eg, there
are two cases to handle:

1 - sc_enabled set.  the first if() is skipped, and the second if()
    is entered, enabled cleared, and maybe close device if open.
2 - sc_enabled not set.  if the first if() is not there at all,
    then the second if() will fail, and nothing happens.  if the
    first if() is there, nothing happens.

i've had a couple of crashes near here recently and i haven't been
able to understand them (or reproduce easily, with USBHIST), so i
was thinking this might help me, but unless i've missed something
above, this doesn't actually change behaviour.  the issue in uts.c
does appear to have been a real problem, perhaps because it didn't
have the 2nd if() that ums_disable() currently does.

(note: the second if() could be removed now, since it will always
be true if the code gets to this point.  oh, which makes it like
the uts.c version.)

thanks.


.mrg.