[{"id":1763055,"web_url":"http://patchwork.ozlabs.org/comment/1763055/","msgid":"<0a344384-f482-5602-490a-928db53c46b5@redhat.com>","list_archive_url":null,"date":"2017-09-05T08:12:30","subject":"Re: [Qemu-devel] [PATCH v6 08/29] libqtest: Let socket_send()\n\tcompute length","submitter":{"id":66152,"url":"http://patchwork.ozlabs.org/api/people/66152/","name":"Thomas Huth","email":"thuth@redhat.com"},"content":"On 01.09.2017 20:03, Eric Blake wrote:\n> Rather than make multiple callers call strlen(), it's easier if\n> socket_send() itself can compute a length via strlen() if none\n> was provided (caller passes -1). Callers that can get at the\n> length more efficiently are left that way.\n> \n> Signed-off-by: Eric Blake \n> ---\n> tests/libqtest.c | 10 ++++++----\n> 1 file changed, 6 insertions(+), 4 deletions(-)\n\nI have to say that I don't like this idea very much. socket_send()\nshould IMHO not know about the type of the data that should be sent,\ni.e. it should not assume that the content is a zero-terminated string.\nThis also could lead to some hard to detect bugs later in case somebody\nis calling the function like this:\n\n size = someotherfunction();\n socket_send(fd, buf, size);\n\n... and the someotherfunction() returned a negative error code instead\nof a correct size.\n\nSo I'd like to suggest to simply drop this patch.\n\n Thomas","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=thuth@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xmfbn2XlZz9s0g\n\tfor ;\n\tTue, 5 Sep 2017 18:13:08 +1000 (AEST)","from localhost ([::1]:57355 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dp8ym-0004EY-9c\n\tfor incoming@patchwork.ozlabs.org; Tue, 05 Sep 2017 04:13:04 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:37847)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dp8yN-0004E6-8N\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 04:12:47 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dp8yI-0003dl-7o\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 04:12:39 -0400","from mx1.redhat.com ([209.132.183.28]:40264)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dp8yI-0003d7-1w\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 04:12:34 -0400","from smtp.corp.redhat.com\n\t(int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id DC26680461\n\tfor ; Tue, 5 Sep 2017 08:12:32 +0000 (UTC)","from [10.36.116.114] (ovpn-116-114.ams2.redhat.com [10.36.116.114])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id B39D3A64BE;\n\tTue, 5 Sep 2017 08:12:31 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com DC26680461","To":"Eric Blake , qemu-devel@nongnu.org","References":"<20170901180340.30009-1-eblake@redhat.com>\n\t<20170901180340.30009-9-eblake@redhat.com>","From":"Thomas Huth ","Message-ID":"<0a344384-f482-5602-490a-928db53c46b5@redhat.com>","Date":"Tue, 5 Sep 2017 10:12:30 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170901180340.30009-9-eblake@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.11","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]);\n\tTue, 05 Sep 2017 08:12:33 +0000 (UTC)","Content-Transfer-Encoding":"quoted-printable","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v6 08/29] libqtest: Let socket_send()\n\tcompute length","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"pbonzini@redhat.com, armbru@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1763164,"web_url":"http://patchwork.ozlabs.org/comment/1763164/","msgid":"<87zia9xyfo.fsf@dusky.pond.sub.org>","list_archive_url":null,"date":"2017-09-05T09:54:51","subject":"Re: [Qemu-devel] [PATCH v6 08/29] libqtest: Let socket_send()\n\tcompute length","submitter":{"id":2645,"url":"http://patchwork.ozlabs.org/api/people/2645/","name":"Markus Armbruster","email":"armbru@redhat.com"},"content":"Thomas Huth writes:\n\n> On 01.09.2017 20:03, Eric Blake wrote:\n>> Rather than make multiple callers call strlen(), it's easier if\n>> socket_send() itself can compute a length via strlen() if none\n>> was provided (caller passes -1). Callers that can get at the\n>> length more efficiently are left that way.\n>> \n>> Signed-off-by: Eric Blake \n>> ---\n>> tests/libqtest.c | 10 ++++++----\n>> 1 file changed, 6 insertions(+), 4 deletions(-)\n>\n> I have to say that I don't like this idea very much. socket_send()\n> should IMHO not know about the type of the data that should be sent,\n> i.e. it should not assume that the content is a zero-terminated string.\n\nI agree.\n\n> This also could lead to some hard to detect bugs later in case somebody\n> is calling the function like this:\n>\n> size = someotherfunction();\n> socket_send(fd, buf, size);\n>\n> ... and the someotherfunction() returned a negative error code instead\n> of a correct size.\n>\n> So I'd like to suggest to simply drop this patch.\n\nA separate wrapper function for sending zero-terminated strings would be\nfine with me.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=armbru@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xmhvD31BCz9s9Y\n\tfor ;\n\tTue, 5 Sep 2017 19:56:40 +1000 (AEST)","from localhost ([::1]:57833 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dpAb0-0004BU-Gc\n\tfor incoming@patchwork.ozlabs.org; Tue, 05 Sep 2017 05:56:38 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:33416)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dpAZO-00034C-GK\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:55:03 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dpAZJ-0001mo-Up\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:54:58 -0400","from mx1.redhat.com ([209.132.183.28]:60120)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dpAZJ-0001lv-P3\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:54:53 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id ADB17356C7\n\tfor ; Tue, 5 Sep 2017 09:54:52 +0000 (UTC)","from blackfin.pond.sub.org (ovpn-116-75.ams2.redhat.com\n\t[10.36.116.75])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 798B684D76;\n\tTue, 5 Sep 2017 09:54:52 +0000 (UTC)","by blackfin.pond.sub.org (Postfix, from userid 1000)\n\tid 41EF21138645; Tue, 5 Sep 2017 11:54:51 +0200 (CEST)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com ADB17356C7","From":"Markus Armbruster ","To":"Thomas Huth ","References":"<20170901180340.30009-1-eblake@redhat.com>\n\t<20170901180340.30009-9-eblake@redhat.com>\n\t<0a344384-f482-5602-490a-928db53c46b5@redhat.com>","Date":"Tue, 05 Sep 2017 11:54:51 +0200","In-Reply-To":"<0a344384-f482-5602-490a-928db53c46b5@redhat.com> (Thomas Huth's\n\tmessage of \"Tue, 5 Sep 2017 10:12:30 +0200\")","Message-ID":"<87zia9xyfo.fsf@dusky.pond.sub.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux)","MIME-Version":"1.0","Content-Type":"text/plain","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tTue, 05 Sep 2017 09:54:52 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v6 08/29] libqtest: Let socket_send()\n\tcompute length","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"pbonzini@redhat.com, qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1763700,"web_url":"http://patchwork.ozlabs.org/comment/1763700/","msgid":"<2316c879-7dc2-c121-1a41-41253c204925@redhat.com>","list_archive_url":null,"date":"2017-09-05T22:20:05","subject":"Re: [Qemu-devel] [PATCH v6 08/29] libqtest: Let socket_send()\n\tcompute length","submitter":{"id":6591,"url":"http://patchwork.ozlabs.org/api/people/6591/","name":"Eric Blake","email":"eblake@redhat.com"},"content":"On 09/05/2017 04:54 AM, Markus Armbruster wrote:\n> Thomas Huth writes:\n> \n>> On 01.09.2017 20:03, Eric Blake wrote:\n>>> Rather than make multiple callers call strlen(), it's easier if\n>>> socket_send() itself can compute a length via strlen() if none\n>>> was provided (caller passes -1). Callers that can get at the\n>>> length more efficiently are left that way.\n>>>\n>>> Signed-off-by: Eric Blake \n>>> ---\n>>> tests/libqtest.c | 10 ++++++----\n>>> 1 file changed, 6 insertions(+), 4 deletions(-)\n>>\n>> I have to say that I don't like this idea very much. socket_send()\n>> should IMHO not know about the type of the data that should be sent,\n>> i.e. it should not assume that the content is a zero-terminated string.\n> \n> I agree.\n\nIt doesn't assume that the content is zero-terminated unless you pass a\nnegative length.\n\n> \n>> This also could lead to some hard to detect bugs later in case somebody\n>> is calling the function like this:\n>>\n>> size = someotherfunction();\n>> socket_send(fd, buf, size);\n>>\n>> ... and the someotherfunction() returned a negative error code instead\n>> of a correct size.\n>>\n>> So I'd like to suggest to simply drop this patch.\n> \n> A separate wrapper function for sending zero-terminated strings would be\n> fine with me.\n\nI'm fine dropping the patch; computing the length in the callers is not\nthat much more onerous (there aren't that many), so I don't think\nanother wrapper is needed.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=eblake@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xn1Pq17LBz9s83\n\tfor ;\n\tWed, 6 Sep 2017 08:20:45 +1000 (AEST)","from localhost ([::1]:33390 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dpMD4-0002Ao-2g\n\tfor incoming@patchwork.ozlabs.org; Tue, 05 Sep 2017 18:20:42 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:48648)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dpMCf-0002AX-E8\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 18:20:22 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dpMCa-0003W0-I8\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 18:20:17 -0400","from mx1.redhat.com ([209.132.183.28]:41534)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dpMCa-0003VZ-8l\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 18:20:12 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 0FE5B81E1D\n\tfor ; Tue, 5 Sep 2017 22:20:11 +0000 (UTC)","from [10.10.120.228] (ovpn-120-228.rdu2.redhat.com [10.10.120.228])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id CF6497EF94;\n\tTue, 5 Sep 2017 22:20:06 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 0FE5B81E1D","To":"Markus Armbruster , Thomas Huth ","References":"<20170901180340.30009-1-eblake@redhat.com>\n\t<20170901180340.30009-9-eblake@redhat.com>\n\t<0a344384-f482-5602-490a-928db53c46b5@redhat.com>\n\t<87zia9xyfo.fsf@dusky.pond.sub.org>","From":"Eric Blake ","Openpgp":"url=http://people.redhat.com/eblake/eblake.gpg","Organization":"Red Hat, Inc.","Message-ID":"<2316c879-7dc2-c121-1a41-41253c204925@redhat.com>","Date":"Tue, 5 Sep 2017 17:20:05 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<87zia9xyfo.fsf@dusky.pond.sub.org>","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"gpCgKw7xAmB3jRbKW4xsEHbd8hUsj9drn\"","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.25]);\n\tTue, 05 Sep 2017 22:20:11 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","X-Content-Filtered-By":"Mailman/MimeDel 2.1.21","Subject":"Re: [Qemu-devel] [PATCH v6 08/29] libqtest: Let socket_send()\n\tcompute length","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"pbonzini@redhat.com, qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]