Question, comment and a potential bug ...
On Wed, Jul 16, 2014 at 04:54:49AM +0000, Doug Hogan wrote:
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/est.c,v
> retrieving revision 1.33
> diff -u -p -d -r1.33 est.c
> --- sys/arch/amd64/amd64/est.c 12 Jul 2014 18:44:41 -0000 1.33
> +++ sys/arch/amd64/amd64/est.c 15 Jul 2014 23:48:21 -0000
[...]
> @@ -381,8 +381,8 @@ est_init(struct cpu_info *ci)
> }
>
>
> - if ((fake_table = malloc(sizeof(struct est_op) * 3, M_DEVBUF,
> - M_NOWAIT)) == NULL) {
> + if ((fake_table = mallocarray(3, sizeof(struct est_op),
> + M_DEVBUF, M_NOWAIT)) == NULL) {
For obvious cases such as this, is it worth converting?
This is almost suggesting following conversion:
- ptr = malloc(sizeof(struct mystruct), ...);
+ ptr = mallocarray(1, sizeof(struct mystruct), ...);
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/i386/est.c,v
> retrieving revision 1.43
> diff -u -p -d -r1.43 est.c
> --- sys/arch/i386/i386/est.c 12 Jul 2014 18:44:41 -0000 1.43
> +++ sys/arch/i386/i386/est.c 15 Jul 2014 23:48:23 -0000
[...]
> @@ -1140,8 +1140,8 @@ est_init(struct cpu_info *ci, int vendor
> }
>
>
> - if ((fake_table = malloc(sizeof(struct est_op) * 3, M_DEVBUF,
> - M_NOWAIT)) == NULL) {
> + if ((fake_table = mallocarray(3, sizeof(struct est_op),
> + M_DEVBUF, M_NOWAIT)) == NULL) {
[...]
> ===================================================================
> RCS file: /cvs/src/sys/arch/sparc/dev/obio.c,v
> retrieving revision 1.22
> diff -u -p -d -r1.22 obio.c
> --- sys/arch/sparc/dev/obio.c 5 Sep 2010 18:10:10 -0000 1.22
> +++ sys/arch/sparc/dev/obio.c 15 Jul 2014 23:48:26 -0000
> @@ -310,7 +310,7 @@ vmesattach(parent, self, args)
> printf("\n");
>
> if (vmeints == NULL) {
> - vmeints = malloc(256 * sizeof(struct intrhand *), M_TEMP,
> + vmeints = mallocarray(256, sizeof(struct intrhand *), M_TEMP,
> M_NOWAIT | M_ZERO);
> if (vmeints == NULL)
> panic("vmesattach: can't allocate intrhand");
> @@ -332,7 +332,7 @@ vmelattach(parent, self, args)
> printf("\n");
>
> if (vmeints == NULL) {
> - vmeints = malloc(256 * sizeof(struct intrhand *), M_TEMP,
> + vmeints = mallocarray(256, sizeof(struct intrhand *), M_TEMP,
> M_NOWAIT | M_ZERO);
> if (vmeints == NULL)
> panic("vmelattach: can't allocate intrhand");
[...]
> Index: sys/arch/sparc64/dev/vdsk.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/sparc64/dev/vdsk.c,v
> retrieving revision 1.39
> diff -u -p -d -r1.39 vdsk.c
> --- sys/arch/sparc64/dev/vdsk.c 12 Jul 2014 18:44:43 -0000 1.39
> +++ sys/arch/sparc64/dev/vdsk.c 15 Jul 2014 23:48:27 -0000
> @@ -298,7 +298,7 @@ vdsk_attach(struct device *parent, struc
> printf(", can't allocate dring\n");
> goto free_map;
> }
> - sc->sc_vsd = malloc(32 * sizeof(*sc->sc_vsd), M_DEVBUF, M_NOWAIT);
> + sc->sc_vsd = mallocarray(32, sizeof(*sc->sc_vsd), M_DEVBUF, M_NOWAIT);
> if (sc->sc_vsd == NULL) {
> printf(", can't allocate software ring\n");
> goto free_dring;
[...]
> Index: sys/arch/sparc64/dev/vnet.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/sparc64/dev/vnet.c,v
> retrieving revision 1.33
> diff -u -p -d -r1.33 vnet.c
> --- sys/arch/sparc64/dev/vnet.c 12 Jul 2014 18:44:43 -0000 1.33
> +++ sys/arch/sparc64/dev/vnet.c 15 Jul 2014 23:48:27 -0000
> @@ -1381,7 +1381,7 @@ vnet_init(struct ifnet *ifp)
> sc->sc_vd = vnet_dring_alloc(sc->sc_dmatag, 128);
> if (sc->sc_vd == NULL)
> return;
> - sc->sc_vsd = malloc(128 * sizeof(*sc->sc_vsd), M_DEVBUF, M_NOWAIT);
> + sc->sc_vsd = mallocarray(128, sizeof(*sc->sc_vsd), M_DEVBUF, M_NOWAIT);
> if (sc->sc_vsd == NULL)
> return;
>
[...]
> Index: sys/dev/usb/uaudio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
> retrieving revision 1.104
> diff -u -p -d -r1.104 uaudio.c
> --- sys/dev/usb/uaudio.c 12 Jul 2014 18:48:52 -0000 1.104
> +++ sys/dev/usb/uaudio.c 15 Jul 2014 23:48:35 -0000
[...]
> @@ -1962,7 +1962,8 @@ uaudio_identify_ac(struct uaudio_softc *
> ibufend = ibuf + aclen;
> dp = (const usb_descriptor_t *)ibuf;
> ndps = 0;
> - iot = malloc(sizeof(struct io_terminal) * 256, M_TEMP, M_NOWAIT |
> M_ZERO);
> + iot = mallocarray(256, sizeof(struct io_terminal), M_TEMP,
> + M_NOWAIT | M_ZERO);
> if (iot == NULL) {
> printf("%s: no memory\n", __func__);
> return USBD_NOMEM;
[...]
> Index: sys/dev/usb/usb_subr.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v
> retrieving revision 1.106
> diff -u -p -d -r1.106 usb_subr.c
> --- sys/dev/usb/usb_subr.c 12 Jul 2014 18:48:53 -0000 1.106
> +++ sys/dev/usb/usb_subr.c 15 Jul 2014 23:48:37 -0000
> @@ -875,7 +875,7 @@ usbd_probe_and_attach(struct device *par
> DPRINTF(("usbd_probe_and_attach trying device specific drivers\n"));
> dv = config_found_sm(parent, &uaa, usbd_print, usbd_submatch);
> if (dv) {
> - dev->subdevs = malloc(2 * sizeof dv, M_USB, M_NOWAIT);
> + dev->subdevs = mallocarray(2, sizeof(dv), M_USB, M_NOWAIT);
> if (dev->subdevs == NULL) {
> err = USBD_NOMEM;
> goto fail;
[...]
> @@ -974,7 +974,8 @@ generic:
> dv = config_found_sm(parent, &uaa, usbd_print, usbd_submatch);
> if (dv != NULL) {
> if (dev->ndevs == 0) {
> - dev->subdevs = malloc(2 * sizeof dv, M_USB, M_NOWAIT);
> + dev->subdevs = mallocarray(2, sizeof(dv), M_USB,
> + M_NOWAIT);
> if (dev->subdevs == NULL) {
> err = USBD_NOMEM;
> goto fail;
[...]
> Index: sys/dev/wscons/wsemul_vt100.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/wscons/wsemul_vt100.c,v
> retrieving revision 1.32
> diff -u -p -d -r1.32 wsemul_vt100.c
> --- sys/dev/wscons/wsemul_vt100.c 12 Jul 2014 18:48:53 -0000 1.32
> +++ sys/dev/wscons/wsemul_vt100.c 15 Jul 2014 23:48:37 -0000
> @@ -222,10 +222,10 @@ wsemul_vt100_attach(int console, const s
> edp->dblwid = malloc(edp->nrows, M_DEVBUF, M_NOWAIT | M_ZERO);
> edp->dw = 0;
> edp->dcsarg = malloc(DCS_MAXLEN, M_DEVBUF, M_NOWAIT);
> - edp->isolatin1tab = malloc(128 * sizeof(u_int), M_DEVBUF, M_NOWAIT);
> - edp->decgraphtab = malloc(128 * sizeof(u_int), M_DEVBUF, M_NOWAIT);
> - edp->dectechtab = malloc(128 * sizeof(u_int), M_DEVBUF, M_NOWAIT);
> - edp->nrctab = malloc(128 * sizeof(u_int), M_DEVBUF, M_NOWAIT);
> + edp->isolatin1tab = mallocarray(128, sizeof(u_int), M_DEVBUF, M_NOWAIT);
> + edp->decgraphtab = mallocarray(128, sizeof(u_int), M_DEVBUF, M_NOWAIT);
> + edp->dectechtab = mallocarray(128, sizeof(u_int), M_DEVBUF, M_NOWAIT);
> + edp->nrctab = mallocarray(128, sizeof(u_int), M_DEVBUF, M_NOWAIT);
> vt100_initchartables(edp);
> wsemul_vt100_reset(edp);
> return (edp);
all same as above.
> Index: sys/arch/sgi/gio/grtwo.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/sgi/gio/grtwo.c,v
> retrieving revision 1.10
> diff -u -p -d -r1.10 grtwo.c
> --- sys/arch/sgi/gio/grtwo.c 12 Jul 2014 18:44:42 -0000 1.10
> +++ sys/arch/sgi/gio/grtwo.c 15 Jul 2014 23:48:25 -0000
> @@ -509,7 +509,7 @@ grtwo_init_screen(struct grtwo_devconfig
> * be able to paint an inverted cursor.
> */
> if (dc->dc_bs == NULL) {
> - dc->dc_bs = malloc(ri->ri_rows * ri->ri_cols *
> + dc->dc_bs = mallocarray(ri->ri_rows * ri->ri_cols,
> sizeof(struct wsdisplay_charcell), M_DEVBUF,
might be safer to change this (in a separate diff) to:
dc->dc_bs = mallocarray(ri->ri_rows,
ri->ri_cols * sizeof(struct wsdisplay_charcell),
...
> Index: sys/dev/ic/mfi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/mfi.c,v
> retrieving revision 1.154
> diff -u -p -d -r1.154 mfi.c
> --- sys/dev/ic/mfi.c 13 Jul 2014 23:10:23 -0000 1.154
> +++ sys/dev/ic/mfi.c 16 Jul 2014 00:13:13 -0000
> @@ -2148,8 +2148,8 @@ mfi_create_sensors(struct mfi_softc *sc)
> sensor_attach(&sc->sc_sensordev, &sc->sc_bbu[i]);
> }
>
> - sc->sc_bbu_status = malloc(sizeof(*sc->sc_bbu_status) *
> - sizeof(mfi_bbu_indicators), M_DEVBUF, M_WAITOK | M_ZERO);
> + sc->sc_bbu_status = mallocarray(sizeof(mfi_bbu_indicators),
> + sizeof(*sc->sc_bbu_status), M_DEVBUF, M_WAITOK | M_ZERO);
>
> for (i = 0; i < nitems(mfi_bbu_indicators); i++) {
> sc->sc_bbu_status[i].type = SENSOR_INDICATOR;
Is the bit of `sizeof(mfi_bbu_indicators)' a bug in
the original source code?
should it not be `nitems(mfi_bbu_indicators)'?
best,
--patrick
