| Message ID | 1457771973-23208-1-git-send-email-dhannawatpooja1@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
could you explain why we need this change ? On 03/12/2016 04:39 PM, Pooja Dhannawat wrote: > Signed-off-by: Pooja Dhannawat <dhannawatpooja1@gmail.com> > --- > net/net.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/net/net.c b/net/net.c > index b0c832e..5399758 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -709,16 +709,18 @@ 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 *buf; > uint8_t *buffer; > size_t offset; > > + buf = g_new(uint8_t, 1); > + > 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)); > + offset = iov_to_buf(iov, iovcnt, 0, (uint8_t *)buf, sizeof(uint8_t)); > } > > if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { > @@ -726,6 +728,8 @@ static ssize_t nc_sendv_compat(NetClientState *nc, const struct iovec *iov, > } else { forget to release 'buf' > return nc->info->receive(nc, buffer, offset); > } > + > + g_free(buf); > } > > ssize_t qemu_deliver_packet_iov(NetClientState *sender, >
On 03/12/2016 01:39 AM, Pooja Dhannawat wrote: > Signed-off-by: Pooja Dhannawat <dhannawatpooja1@gmail.com> > --- > net/net.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/net/net.c b/net/net.c > index b0c832e..5399758 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -709,16 +709,18 @@ 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 *buf; > uint8_t *buffer; > size_t offset; > > + buf = g_new(uint8_t, 1); If you're only going to malloc() one byte, it's more efficient to just stack-allocate it: uint8_t buf[1]; Did you mean to do: buf = g_new(uint8_t, NET_BUFSIZE) instead? > + > 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)); > + offset = iov_to_buf(iov, iovcnt, 0, (uint8_t *)buf, sizeof(uint8_t)); This is wrong. The old code used NET_BUFSIZE bytes for iov_to_buf(), the new code uses only one byte. By the way, sizeof(char) and sizeof(uint8_t) are pointless in code, as they are strictly equivalent to 1. I agree that the idea behind the patch is useful (NET_BUFSIZE is 68k, which is too large for our goal of at most 4k stack allocation if we never want to overflow a stack guard page), but you'll need a correct working version of the patch.
On Mon, Mar 14, 2016 at 7:41 PM, Eric Blake <eblake@redhat.com> wrote: > On 03/12/2016 01:39 AM, Pooja Dhannawat wrote: > > Signed-off-by: Pooja Dhannawat <dhannawatpooja1@gmail.com> > > --- > > net/net.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/net/net.c b/net/net.c > > index b0c832e..5399758 100644 > > --- a/net/net.c > > +++ b/net/net.c > > @@ -709,16 +709,18 @@ 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 *buf; > > uint8_t *buffer; > > size_t offset; > > > > + buf = g_new(uint8_t, 1); > > If you're only going to malloc() one byte, it's more efficient to just > stack-allocate it: > > uint8_t buf[1]; > > Did you mean to do: > > buf = g_new(uint8_t, NET_BUFSIZE) > > instead? > > Yes, my bad, I meant NET_BUFSIZE. > > + > > 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)); > > + offset = iov_to_buf(iov, iovcnt, 0, (uint8_t *)buf, > sizeof(uint8_t)); > > This is wrong. The old code used NET_BUFSIZE bytes for iov_to_buf(), > the new code uses only one byte. By the way, sizeof(char) and > sizeof(uint8_t) are pointless in code, as they are strictly equivalent to > 1. > > I agree that the idea behind the patch is useful (NET_BUFSIZE is 68k, > which is too large for our goal of at most 4k stack allocation if we > never want to overflow a stack guard page), but you'll need a correct > working version of the patch. > > Yes, I will correct it and send next version of patch. > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > >
diff --git a/net/net.c b/net/net.c index b0c832e..5399758 100644 --- a/net/net.c +++ b/net/net.c @@ -709,16 +709,18 @@ 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 *buf; uint8_t *buffer; size_t offset; + buf = g_new(uint8_t, 1); + 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)); + offset = iov_to_buf(iov, iovcnt, 0, (uint8_t *)buf, sizeof(uint8_t)); } if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { @@ -726,6 +728,8 @@ static ssize_t nc_sendv_compat(NetClientState *nc, const struct iovec *iov, } else { return nc->info->receive(nc, buffer, offset); } + + g_free(buf); } ssize_t qemu_deliver_packet_iov(NetClientState *sender,
Signed-off-by: Pooja Dhannawat <dhannawatpooja1@gmail.com> --- net/net.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)