On Fri, Nov 11, 2022 at 06:48:21AM +0000, Miod Vallat wrote: > > This actually breaks my machine. malloc() is saying allocation too > > large. OF_getproplen will return -1 on that. Is it possible that > > len is treated as uint64_t as it is an int and sizeof is effectively > > uint64_t? > > Ah, yes; size_t is unsigned and wider than int on 64-bit platforms, > therefore int is converted to unsigned for the comparison. Casting > sizeof to int will do. > > > Might be easier to have a check like: > > > > if (sc->sc_channels == NULL) > > return level < sc->sc_nlevels ? level : sc->sc_nlevels - 1; > > > > Then you don't need to indent the whole block. Makes the diff smaller > > and a bit easier to understand? > > Sure; what about this new version, then?
Works for me, thanks! ok patrick@ > > Index: pwmbl.c > =================================================================== > RCS file: /OpenBSD/src/sys/dev/fdt/pwmbl.c,v > retrieving revision 1.6 > diff -u -p -r1.6 pwmbl.c > --- pwmbl.c 24 Oct 2021 17:52:26 -0000 1.6 > +++ pwmbl.c 11 Nov 2022 06:46:41 -0000 > @@ -35,7 +35,7 @@ struct pwmbl_softc { > struct device sc_dev; > uint32_t *sc_pwm; > int sc_pwm_len; > - uint32_t *sc_levels; > + uint32_t *sc_levels; /* NULL if simple ramp */ > int sc_nlevels; > uint32_t sc_max_level; > uint32_t sc_def_level; > @@ -73,7 +73,7 @@ pwmbl_attach(struct device *parent, stru > struct pwmbl_softc *sc = (struct pwmbl_softc *)self; > struct fdt_attach_args *faa = aux; > uint32_t *gpios; > - int i, len; > + int len; > > len = OF_getproplen(faa->fa_node, "pwms"); > if (len < 0) { > @@ -95,7 +95,7 @@ pwmbl_attach(struct device *parent, stru > } > > len = OF_getproplen(faa->fa_node, "brightness-levels"); > - if (len > 0) { > + if (len >= (int)sizeof(uint32_t)) { > sc->sc_levels = malloc(len, M_DEVBUF, M_WAITOK); > OF_getpropintarray(faa->fa_node, "brightness-levels", > sc->sc_levels, len); > @@ -107,13 +107,9 @@ pwmbl_attach(struct device *parent, stru > sc->sc_def_level = sc->sc_nlevels - 1; > sc->sc_def_level = sc->sc_levels[sc->sc_def_level]; > } else { > + /* No levels, assume a simple 0..255 ramp. */ > sc->sc_nlevels = 256; > - sc->sc_levels = mallocarray(sc->sc_nlevels, > - sizeof(uint32_t), M_DEVBUF, M_WAITOK); > - for (i = 0; i < sc->sc_nlevels; i++) > - sc->sc_levels[i] = i; > - sc->sc_max_level = sc->sc_levels[sc->sc_nlevels - 1]; > - sc->sc_def_level = sc->sc_levels[sc->sc_nlevels - 1]; > + sc->sc_max_level = sc->sc_def_level = sc->sc_nlevels - 1; > } > > printf("\n"); > @@ -143,6 +139,9 @@ pwmbl_find_brightness(struct pwmbl_softc > { > uint32_t mid; > int i; > + > + if (sc->sc_levels == NULL) > + return level < sc->sc_nlevels ? level : sc->sc_nlevels - 1; > > for (i = 0; i < sc->sc_nlevels - 1; i++) { > mid = (sc->sc_levels[i] + sc->sc_levels[i + 1]) / 2; >