On Mon, Mar 23, 2020 at 10:51:06PM +0100, Tobias Heider wrote:
> In alloc_all_jacks() the variables 'sc_in_jacks' and 'sc_out_checks'
> are set to NULL if 'sc_in_num_jacks' and 'sc_out_num_jacks' are 0.
> Further down both are dereferenced unconditionally. I added explicit NULL
> checks where I think they belong.
> I think 'sc_in_ep' and 'sc_out_ep' can also be NULL, but I am not
> entirely sure about those.
>
> Objections or ok?
>
sc_{out,in}_jacks is used only if sc_out_num_jacks > 0, i.e. if
there's at least one iteration. But if sc_out_num_jacks > 0, then
sc_{out,in}_jacks is properly initialized by this chunk:
sc->sc_out_jacks =
sc->sc_out_num_jacks ? sc->sc_jacks : NULL;
sc->sc_in_jacks =
sc->sc_in_num_jacks ? sc->sc_jacks + sc->sc_out_num_jacks : NULL;
AFAICS, the code is correct; but the style looks misleading to me.
> Index: dev/usb/umidi.c
> ===================================================================
> RCS file: /mount/openbsd/cvs/src/sys/dev/usb/umidi.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 umidi.c
> --- dev/usb/umidi.c 16 Mar 2020 16:12:43 -0000 1.53
> +++ dev/usb/umidi.c 23 Mar 2020 21:50:49 -0000
> @@ -724,50 +724,58 @@ alloc_all_jacks(struct umidi_softc *sc)
> sc->sc_in_jacks =
> sc->sc_in_num_jacks ? sc->sc_jacks+sc->sc_out_num_jacks : NULL;
>
> - jack = &sc->sc_out_jacks[0];
> - for (i=0; i<sc->sc_out_num_jacks; i++) {
> - jack->opened = 0;
> - jack->binded = 0;
> - jack->arg = NULL;
> - jack->u.out.intr = NULL;
> - jack->intr = 0;
> - jack->cable_number = i;
> - jack++;
> + if (sc->sc_out_jacks) {
> + jack = &sc->sc_out_jacks[0];
> + for (i=0; i<sc->sc_out_num_jacks; i++) {
> + jack->opened = 0;
> + jack->binded = 0;
> + jack->arg = NULL;
> + jack->u.out.intr = NULL;
> + jack->intr = 0;
> + jack->cable_number = i;
> + jack++;
> + }
> }
> - jack = &sc->sc_in_jacks[0];
> - for (i=0; i<sc->sc_in_num_jacks; i++) {
> - jack->opened = 0;
> - jack->binded = 0;
> - jack->arg = NULL;
> - jack->u.in.intr = NULL;
> - jack->cable_number = i;
> - jack++;
> + if (sc->sc_in_jacks) {
> + jack = &sc->sc_in_jacks[0];
> + for (i=0; i<sc->sc_in_num_jacks; i++) {
> + jack->opened = 0;
> + jack->binded = 0;
> + jack->arg = NULL;
> + jack->u.in.intr = NULL;
> + jack->cable_number = i;
> + jack++;
> + }
> }
>
> /* assign each jacks to each endpoints */
> - jack = &sc->sc_out_jacks[0];
> - ep = &sc->sc_out_ep[0];
> - for (i=0; i<sc->sc_out_num_endpoints; i++) {
> - rjack = &ep->jacks[0];
> - for (j=0; j<ep->num_jacks; j++) {
> - *rjack = jack;
> - jack->endpoint = ep;
> - jack++;
> - rjack++;
> + if (sc->sc_out_jacks && sc->sc_out_ep) {
> + jack = &sc->sc_out_jacks[0];
> + ep = &sc->sc_out_ep[0];
> + for (i=0; i<sc->sc_out_num_endpoints; i++) {
> + rjack = &ep->jacks[0];
> + for (j=0; j<ep->num_jacks; j++) {
> + *rjack = jack;
> + jack->endpoint = ep;
> + jack++;
> + rjack++;
> + }
> + ep++;
> }
> - ep++;
> }
> - jack = &sc->sc_in_jacks[0];
> - ep = &sc->sc_in_ep[0];
> - for (i=0; i<sc->sc_in_num_endpoints; i++) {
> - rjack = &ep->jacks[0];
> - for (j=0; j<ep->num_jacks; j++) {
> - *rjack = jack;
> - jack->endpoint = ep;
> - jack++;
> - rjack++;
> + if (sc->sc_in_jacks && sc->sc_in_ep) {
> + jack = &sc->sc_in_jacks[0];
> + ep = &sc->sc_in_ep[0];
> + for (i=0; i<sc->sc_in_num_endpoints; i++) {
> + rjack = &ep->jacks[0];
> + for (j=0; j<ep->num_jacks; j++) {
> + *rjack = jack;
> + jack->endpoint = ep;
> + jack++;
> + rjack++;
> + }
> + ep++;
> }
> - ep++;
> }
>
> return USBD_NORMAL_COMPLETION;
>
