From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44410) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afUTf-0004IQ-MO for qemu-devel@nongnu.org; Mon, 14 Mar 2016 11:32:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1afUTb-0007uM-0t for qemu-devel@nongnu.org; Mon, 14 Mar 2016 11:32:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35893) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afUTa-0007tg-NM for qemu-devel@nongnu.org; Mon, 14 Mar 2016 11:32:10 -0400 References: <1457959602-6472-1-git-send-email-aesmade@gmail.com> From: Eric Blake Message-ID: <56E6D979.5000900@redhat.com> Date: Mon, 14 Mar 2016 09:32:09 -0600 MIME-Version: 1.0 In-Reply-To: <1457959602-6472-1-git-send-email-aesmade@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="JcwcFU7GN7PQD2EFVFLbV1ab8vEkn3ne8" Subject: Re: [Qemu-devel] [PATCH v3] net.c: Moved large array in nc_sendv_compat from the stack to the heap List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikos Filippakis , qemu-devel@nongnu.org Cc: jasowang@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --JcwcFU7GN7PQD2EFVFLbV1ab8vEkn3ne8 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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). >=20 > Signed-off-by: Nikos Filippakis > --- > net/net.c | 32 +++++++++++++++++++++++++------- > 1 file changed, 25 insertions(+), 7 deletions(-) >=20 > 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 =3D 2048; > + > + uint8_t *buffer, *dynbuf =3D NULL; > + uint8_t stackbuf[STACKBUF_SIZE]; > + size_t offset, iov_len; > + ssize_t ret; > =20 > if (iovcnt =3D=3D 1) { > buffer =3D iov[0].iov_base; > offset =3D iov[0].iov_len; > } else { > - buffer =3D buf; > - offset =3D iov_to_buf(iov, iovcnt, 0, buf, sizeof(buf)); > + iov_len =3D iov_size(iov, iovcnt); > + > + if (iov_len <=3D STACKBUF_SIZE) { > + buffer =3D stackbuf; > + } else if (iov_len <=3D NET_BUFSIZE) { > + buffer =3D dynbuf =3D g_new(uint8_t, iov_len); > + } else { > + errno =3D 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 =3D iov_to_buf(iov, iovcnt, 0, buffer, > + NET_BUFSIZE * sizeof(uint8_t)); sizeof(uint8_t) =3D=3D 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. > } > =20 > if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { > - return nc->info->receive_raw(nc, buffer, offset); > + ret =3D nc->info->receive_raw(nc, buffer, offset); > } else { > - return nc->info->receive(nc, buffer, offset); > + ret =3D nc->info->receive(nc, buffer, offset); > } > + > + g_free(dynbuf); > + > + return ret; > } > =20 > ssize_t qemu_deliver_packet_iov(NetClientState *sender, >=20 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --JcwcFU7GN7PQD2EFVFLbV1ab8vEkn3ne8 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJW5tl5AAoJEKeha0olJ0NqXWcH/0jH9yyBCPg1P8ppGssCoYL3 eJfdhK+vvuAkFhKviUcIFa8DwdFGbSFtVoXFJakcErSOwhhpBxQnbs3xF/hIc6LB wOekidP1di76yyfdd6Gh4a1BI7rFxV29KXdqaJ491wRygjmSZIlEGl9U4InCOuZ3 bbaaJvrkPfbkh2//UnE2vB17xJpfO0ZCS+l6QIL6Q+tcfvBqQ6qRo4m6zp2v0IOM F0vz6VgbyNz3C3T81udj81m/iRAXrvYUP+8/WgnlxHPEtPaTT4Oq1ElmchPiPwAP JozYT8cwF10ZYyvsf6o3htNQuRRQFBnsqiFpTAcTHLbvIk6LQMFlIJ+HKyKXggo= =px6r -----END PGP SIGNATURE----- --JcwcFU7GN7PQD2EFVFLbV1ab8vEkn3ne8--