| Message ID | 1457959602-6472-1-git-send-email-aesmade@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 03/14/2016 06:46 AM, Nikos Filippakis wrote: > Allocate array in nc_sendv_compat on the heap if it is large to reduce stack frame size, as stated in the BiteSizedTasks wiki page. Please wrap your commit messages at 70 or so columns (since 'git log' will display your text with indentation, and many people still prefer 80-column terminal windows). > > Signed-off-by: Nikos Filippakis <aesmade@gmail.com> > --- > net/net.c | 32 +++++++++++++++++++++++++------- > 1 file changed, 25 insertions(+), 7 deletions(-) > > diff --git a/net/net.c b/net/net.c > index b0c832e..a9be287 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -709,23 +709,41 @@ ssize_t qemu_send_packet_raw(NetClientState *nc, const uint8_t *buf, int size) > static ssize_t nc_sendv_compat(NetClientState *nc, const struct iovec *iov, > int iovcnt, unsigned flags) > { > - uint8_t buf[NET_BUFSIZE]; > - uint8_t *buffer; > - size_t offset; > + const size_t STACKBUF_SIZE = 2048; > + > + uint8_t *buffer, *dynbuf = NULL; > + uint8_t stackbuf[STACKBUF_SIZE]; > + size_t offset, iov_len; > + ssize_t ret; > > if (iovcnt == 1) { > buffer = iov[0].iov_base; > offset = iov[0].iov_len; > } else { > - buffer = buf; > - offset = iov_to_buf(iov, iovcnt, 0, buf, sizeof(buf)); > + iov_len = iov_size(iov, iovcnt); > + > + if (iov_len <= STACKBUF_SIZE) { > + buffer = stackbuf; > + } else if (iov_len <= NET_BUFSIZE) { > + buffer = dynbuf = g_new(uint8_t, iov_len); > + } else { > + errno = EINVAL; > + return 0; Is 0 the correct thing to return on an error? Why not -1 or negative errno? Or is this a sign of a coding error, in which case it could be an assertion instead of a special return? > + } > + > + offset = iov_to_buf(iov, iovcnt, 0, buffer, > + NET_BUFSIZE * sizeof(uint8_t)); sizeof(uint8_t) == 1, so this is simpler to write as just: iov_to_buf(iov, iovcnt, 0, buffer, NET_BUFSIZE) except that NET_BUFSIZE is not the right size any more. You want to use iov_len, which is the guaranteed length of buffer, and not NET_BUFSIZE which might be larger than the buffer. > } > > if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { > - return nc->info->receive_raw(nc, buffer, offset); > + ret = nc->info->receive_raw(nc, buffer, offset); > } else { > - return nc->info->receive(nc, buffer, offset); > + ret = nc->info->receive(nc, buffer, offset); > } > + > + g_free(dynbuf); > + > + return ret; > } > > ssize_t qemu_deliver_packet_iov(NetClientState *sender, >
On 14/03/2016 16:32, Eric Blake wrote: >> + const size_t STACKBUF_SIZE = 2048; >> + >> + uint8_t *buffer, *dynbuf = NULL; >> + uint8_t stackbuf[STACKBUF_SIZE]; Instead of using the "STACKBUF_SIZE" constant, you can use just "stackbuf[2048]", and then use sizeof(stackbuf) below. This is even more future proof. Paolo
diff --git a/net/net.c b/net/net.c index b0c832e..a9be287 100644 --- a/net/net.c +++ b/net/net.c @@ -709,23 +709,41 @@ ssize_t qemu_send_packet_raw(NetClientState *nc, const uint8_t *buf, int size) static ssize_t nc_sendv_compat(NetClientState *nc, const struct iovec *iov, int iovcnt, unsigned flags) { - uint8_t buf[NET_BUFSIZE]; - uint8_t *buffer; - size_t offset; + const size_t STACKBUF_SIZE = 2048; + + uint8_t *buffer, *dynbuf = NULL; + uint8_t stackbuf[STACKBUF_SIZE]; + size_t offset, iov_len; + ssize_t ret; if (iovcnt == 1) { buffer = iov[0].iov_base; offset = iov[0].iov_len; } else { - buffer = buf; - offset = iov_to_buf(iov, iovcnt, 0, buf, sizeof(buf)); + iov_len = iov_size(iov, iovcnt); + + if (iov_len <= STACKBUF_SIZE) { + buffer = stackbuf; + } else if (iov_len <= NET_BUFSIZE) { + buffer = dynbuf = g_new(uint8_t, iov_len); + } else { + errno = EINVAL; + return 0; + } + + offset = iov_to_buf(iov, iovcnt, 0, buffer, + NET_BUFSIZE * sizeof(uint8_t)); } if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { - return nc->info->receive_raw(nc, buffer, offset); + ret = nc->info->receive_raw(nc, buffer, offset); } else { - return nc->info->receive(nc, buffer, offset); + ret = nc->info->receive(nc, buffer, offset); } + + g_free(dynbuf); + + return ret; } ssize_t qemu_deliver_packet_iov(NetClientState *sender,
Allocate array in nc_sendv_compat on the heap if it is large to reduce stack frame size, as stated in the BiteSizedTasks wiki page. Signed-off-by: Nikos Filippakis <aesmade@gmail.com> --- net/net.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-)