On Wed, Aug 31, 2016 at 06:04:22AM -0600, nayden wrote: > Addressing "this function needs a cleanup block, lots of free(blah); return > (0)" > comment for vioblk_notifyq() and doing the same for vionet_enq_rx(). >
go for it, thanks. > Index: virtio.c > =================================================================== > RCS file: /home/nayden/cvsync/src/usr.sbin/vmd/virtio.c,v > retrieving revision 1.17 > diff -u -p -r1.17 virtio.c > --- virtio.c 17 Aug 2016 05:07:13 -0000 1.17 > +++ virtio.c 31 Aug 2016 10:35:55 -0000 > @@ -353,8 +353,7 @@ vioblk_do_write(struct vioblk_dev *dev, > } > > /* > - * XXX this function needs a cleanup block, lots of free(blah); return (0) > - * in various cases, ds should be set to VIRTIO_BLK_S_IOERR, if we can > + * XXX in various cases, ds should be set to VIRTIO_BLK_S_IOERR, if we can > * XXX cant trust ring data from VM, be extra cautious. > */ > int > @@ -390,8 +389,7 @@ vioblk_notifyq(struct vioblk_dev *dev) > > if (read_mem(q_gpa, vr, vr_sz)) { > log_warnx("error reading gpa 0x%llx", q_gpa); > - free(vr); > - return (0); > + goto out; > } > > /* Compute offsets in ring of descriptors, avail ring, and used ring */ > @@ -406,8 +404,7 @@ vioblk_notifyq(struct vioblk_dev *dev) > > if ((avail->idx & VIOBLK_QUEUE_MASK) == idx) { > log_warnx("vioblk queue notify - nothing to do?"); > - free(vr); > - return (0); > + goto out; > } > > cmd_desc_idx = avail->ring[idx] & VIOBLK_QUEUE_MASK; > @@ -416,16 +413,14 @@ vioblk_notifyq(struct vioblk_dev *dev) > if ((cmd_desc->flags & VRING_DESC_F_NEXT) == 0) { > log_warnx("unchained vioblk cmd descriptor received " > "(idx %d)", cmd_desc_idx); > - free(vr); > - return (0); > + goto out; > } > > /* Read command from descriptor ring */ > if (read_mem(cmd_desc->addr, &cmd, cmd_desc->len)) { > log_warnx("vioblk: command read_mem error @ 0x%llx", > cmd_desc->addr); > - free(vr); > - return (0); > + goto out; > } > > switch (cmd.type) { > @@ -437,8 +432,7 @@ vioblk_notifyq(struct vioblk_dev *dev) > if ((secdata_desc->flags & VRING_DESC_F_NEXT) == 0) { > log_warnx("unchained vioblk data descriptor " > "received (idx %d)", cmd_desc_idx); > - free(vr); > - return (0); > + goto out; > } > > secbias = 0; > @@ -453,8 +447,7 @@ vioblk_notifyq(struct vioblk_dev *dev) > if (secdata == NULL) { > log_warnx("vioblk: block read error, " > "sector %lld", cmd.sector); > - free(vr); > - return (0); > + goto out; > } > > if (write_mem(secdata_desc->addr, secdata, > @@ -463,9 +456,8 @@ vioblk_notifyq(struct vioblk_dev *dev) > "data to gpa @ 0x%llx", > secdata_desc->addr); > dump_descriptor_chain(desc, cmd_desc_idx); > - free(vr); > free(secdata); > - return (0); > + goto out; > } > > free(secdata); > @@ -484,8 +476,7 @@ vioblk_notifyq(struct vioblk_dev *dev) > log_warnx("can't write device status data @ " > "0x%llx", ds_desc->addr); > dump_descriptor_chain(desc, cmd_desc_idx); > - free(vr); > - return (0); > + goto out; > } > > > @@ -509,16 +500,14 @@ vioblk_notifyq(struct vioblk_dev *dev) > if ((secdata_desc->flags & VRING_DESC_F_NEXT) == 0) { > log_warnx("wr vioblk: unchained vioblk data " > "descriptor received (idx %d)", cmd_desc_idx); > - free(vr); > - return (0); > + goto out; > } > > secdata = malloc(MAXPHYS); > if (secdata == NULL) { > log_warn("wr vioblk: malloc error, len %d", > secdata_desc->len); > - free(vr); > - return (0); > + goto out; > } > > secbias = 0; > @@ -529,17 +518,15 @@ vioblk_notifyq(struct vioblk_dev *dev) > "sector data @ 0x%llx", > secdata_desc->addr); > dump_descriptor_chain(desc, cmd_desc_idx); > - free(vr); > free(secdata); > - return (0); > + goto out; > } > > if (vioblk_do_write(dev, cmd.sector + secbias, > secdata, (ssize_t)secdata_desc->len)) { > log_warnx("wr vioblk: disk write error"); > - free(vr); > free(secdata); > - return (0); > + goto out; > } > > secbias += secdata_desc->len / VIRTIO_BLK_SECTOR_SIZE; > @@ -559,8 +546,7 @@ vioblk_notifyq(struct vioblk_dev *dev) > log_warnx("wr vioblk: can't write device status " > "data @ 0x%llx", ds_desc->addr); > dump_descriptor_chain(desc, cmd_desc_idx); > - free(vr); > - return (0); > + goto out; > } > > ret = 1; > @@ -584,8 +570,7 @@ vioblk_notifyq(struct vioblk_dev *dev) > log_warnx("fl vioblk: can't write device status " > "data @ 0x%llx", ds_desc->addr); > dump_descriptor_chain(desc, cmd_desc_idx); > - free(vr); > - return (0); > + goto out; > } > > ret = 1; > @@ -601,9 +586,8 @@ vioblk_notifyq(struct vioblk_dev *dev) > } > break; > } > - > +out: > free(vr); > - > return (ret); > } > > @@ -810,8 +794,7 @@ vionet_enq_rx(struct vionet_dev *dev, ch > > if (read_mem(q_gpa, vr, vr_sz)) { > log_warnx("rx enq: error reading gpa 0x%llx", q_gpa); > - free(vr); > - return (0); > + goto out; > } > > /* Compute offsets in ring of descriptors, avail ring, and used ring */ > @@ -825,8 +808,7 @@ vionet_enq_rx(struct vionet_dev *dev, ch > > if ((dev->vq[0].notified_avail & VIONET_QUEUE_MASK) == idx) { > log_warnx("vionet queue notify - no space, dropping packet"); > - free(vr); > - return (0); > + goto out; > } > > hdr_desc_idx = avail->ring[idx] & VIONET_QUEUE_MASK; > @@ -839,16 +821,14 @@ vionet_enq_rx(struct vionet_dev *dev, ch > if ((pkt_desc->flags & VRING_DESC_F_WRITE) == 0) { > log_warnx("unexpected readable rx descriptor %d", > pkt_desc_idx); > - free(vr); > - return (0); > + goto out; > } > > /* Write packet to descriptor ring */ > if (write_mem(pkt_desc->addr, pkt, sz)) { > log_warnx("vionet: rx enq packet write_mem error @ " > "0x%llx", pkt_desc->addr); > - free(vr); > - return (0); > + goto out; > } > > ret = 1; > @@ -868,9 +848,8 @@ vionet_enq_rx(struct vionet_dev *dev, ch > if (write_mem(q_gpa + off, &used->idx, sizeof used->idx)) > log_warnx("vionet: error writing vio ring"); > } > - > +out: > free(vr); > - > return (ret); > } > >