From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:56041) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gwPoi-0007tD-Lr for qemu-devel@nongnu.org; Wed, 20 Feb 2019 06:13:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gwPog-0006tn-PL for qemu-devel@nongnu.org; Wed, 20 Feb 2019 06:13:32 -0500 Received: from mail-wr1-f66.google.com ([209.85.221.66]:33663) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gwPog-0006qD-Hs for qemu-devel@nongnu.org; Wed, 20 Feb 2019 06:13:30 -0500 Received: by mail-wr1-f66.google.com with SMTP id i12so25567062wrw.0 for ; Wed, 20 Feb 2019 03:13:29 -0800 (PST) References: <20190220010232.18731-1-philmd@redhat.com> <20190220010232.18731-3-philmd@redhat.com> From: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= Message-ID: <68f7233c-8b95-8782-27a7-106fc2997646@redhat.com> Date: Wed, 20 Feb 2019 12:13:25 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v3 02/25] chardev: Assert IOCanReadHandler can not be negative List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= Cc: qemu-devel , Prasad J Pandit , Paolo Bonzini , Jason Wang , Anthony Perard , qemu-ppc@nongnu.org, Stefan Berger , David Gibson , Gerd Hoffmann , Zhang Chen , xen-devel@lists.xenproject.org, Cornelia Huck , Samuel Thibault , Christian Borntraeger , Amit Shah , Li Zhijian , Corey Minyard , "Michael S. Tsirkin" , Paul Durrant , Halil Pasic , Stefano Stabellini , qemu-s390x@nongnu.org, Pavel Dovgalyuk On 2/20/19 11:03 AM, Marc-André Lureau wrote: > Hi > > On Wed, Feb 20, 2019 at 2:03 AM Philippe Mathieu-Daudé > wrote: >> >> The backend should not return a negative length to read. >> We will later change the prototype of IOCanReadHandler to return an >> unsigned length. Meanwhile make sure the return length is positive. >> >> Suggested-by: Paolo Bonzini >> Signed-off-by: Philippe Mathieu-Daudé > > In such patch, you should do extensive review of existing callbacks, > or find a convincing argument that this can't break. Argh I missed that. > The problem is there are a lot of can_read callbacks, and it's not > trivial. The *first* of git-grep is rng_egd_chr_can_read() > > 57 QSIMPLEQ_FOREACH(req, &s->parent.requests, next) { > 58 size += req->size - req->offset; > 59 } > 60 > 61 return size; > > Clearly not obvious if it returns >= 0. > > Another approach is to look at the caller and the return value > handling. If none handle negative values (or would have wrong > behaviour with negative values), the assert() is perhaps justified, as > it could prevent from doing more harm. I'll go and audit all of them. Thanks for the review! Phil. >> --- >> chardev/char.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/chardev/char.c b/chardev/char.c >> index f6d61fa5f8..71ecd32b25 100644 >> --- a/chardev/char.c >> +++ b/chardev/char.c >> @@ -159,12 +159,15 @@ int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all) >> int qemu_chr_be_can_write(Chardev *s) >> { >> CharBackend *be = s->be; >> + int receivable_bytes; >> >> if (!be || !be->chr_can_read) { >> return 0; >> } >> >> - return be->chr_can_read(be->opaque); >> + receivable_bytes = be->chr_can_read(be->opaque); >> + assert(receivable_bytes >= 0); >> + return receivable_bytes; >> } >> >> void qemu_chr_be_write_impl(Chardev *s, uint8_t *buf, int len) >> -- >> 2.20.1 >> From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= Subject: Re: [PATCH v3 02/25] chardev: Assert IOCanReadHandler can not be negative Date: Wed, 20 Feb 2019 12:13:25 +0100 Message-ID: <68f7233c-8b95-8782-27a7-106fc2997646@redhat.com> References: <20190220010232.18731-1-philmd@redhat.com> <20190220010232.18731-3-philmd@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from us1-rack-dfw2.inumbo.com ([104.130.134.6]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1gwPog-0005cI-G1 for xen-devel@lists.xenproject.org; Wed, 20 Feb 2019 11:13:30 +0000 Received: by mail-wr1-f65.google.com with SMTP id c8so25482626wrs.4 for ; Wed, 20 Feb 2019 03:13:29 -0800 (PST) In-Reply-To: Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" To: =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= Cc: Li Zhijian , "Michael S. Tsirkin" , Jason Wang , qemu-devel , Gerd Hoffmann , Stefano Stabellini , Samuel Thibault , Halil Pasic , Christian Borntraeger , Anthony Perard , xen-devel@lists.xenproject.org, Corey Minyard , Amit Shah , qemu-s390x@nongnu.org, Paul Durrant , Pavel Dovgalyuk , Zhang Chen , David Gibson , Prasad J Pandit , Cornelia Huck , qemu-ppc@nongnu.org, Paolo Bonzini , Stefan Berger List-Id: xen-devel@lists.xenproject.org T24gMi8yMC8xOSAxMTowMyBBTSwgTWFyYy1BbmRyw6kgTHVyZWF1IHdyb3RlOgo+IEhpCj4gCj4g T24gV2VkLCBGZWIgMjAsIDIwMTkgYXQgMjowMyBBTSBQaGlsaXBwZSBNYXRoaWV1LURhdWTDqQo+ IDxwaGlsbWRAcmVkaGF0LmNvbT4gd3JvdGU6Cj4+Cj4+IFRoZSBiYWNrZW5kIHNob3VsZCBub3Qg cmV0dXJuIGEgbmVnYXRpdmUgbGVuZ3RoIHRvIHJlYWQuCj4+IFdlIHdpbGwgbGF0ZXIgY2hhbmdl IHRoZSBwcm90b3R5cGUgb2YgSU9DYW5SZWFkSGFuZGxlciB0byByZXR1cm4gYW4KPj4gdW5zaWdu ZWQgbGVuZ3RoLiBNZWFud2hpbGUgbWFrZSBzdXJlIHRoZSByZXR1cm4gbGVuZ3RoIGlzIHBvc2l0 aXZlLgo+Pgo+PiBTdWdnZXN0ZWQtYnk6IFBhb2xvIEJvbnppbmkgPHBib256aW5pQHJlZGhhdC5j b20+Cj4+IFNpZ25lZC1vZmYtYnk6IFBoaWxpcHBlIE1hdGhpZXUtRGF1ZMOpIDxwaGlsbWRAcmVk aGF0LmNvbT4KPiAKPiBJbiBzdWNoIHBhdGNoLCB5b3Ugc2hvdWxkIGRvIGV4dGVuc2l2ZSByZXZp ZXcgb2YgZXhpc3RpbmcgY2FsbGJhY2tzLAo+IG9yIGZpbmQgYSBjb252aW5jaW5nIGFyZ3VtZW50 IHRoYXQgdGhpcyBjYW4ndCBicmVhay4KCkFyZ2ggSSBtaXNzZWQgdGhhdC4KCj4gVGhlIHByb2Js ZW0gaXMgdGhlcmUgYXJlIGEgbG90IG9mIGNhbl9yZWFkIGNhbGxiYWNrcywgYW5kIGl0J3Mgbm90 Cj4gdHJpdmlhbC4gVGhlICpmaXJzdCogb2YgZ2l0LWdyZXAgaXMgcm5nX2VnZF9jaHJfY2FuX3Jl YWQoKQo+IAo+ICA1NyAgICAgUVNJTVBMRVFfRk9SRUFDSChyZXEsICZzLT5wYXJlbnQucmVxdWVz dHMsIG5leHQpIHsKPiAgNTggICAgICAgICBzaXplICs9IHJlcS0+c2l6ZSAtIHJlcS0+b2Zmc2V0 Owo+ICA1OSAgICAgfQo+ICA2MAo+ICA2MSAgICAgcmV0dXJuIHNpemU7Cj4gCj4gQ2xlYXJseSBu b3Qgb2J2aW91cyBpZiBpdCByZXR1cm5zID49IDAuCj4gCj4gQW5vdGhlciBhcHByb2FjaCBpcyB0 byBsb29rIGF0IHRoZSBjYWxsZXIgYW5kIHRoZSByZXR1cm4gdmFsdWUKPiBoYW5kbGluZy4gSWYg bm9uZSBoYW5kbGUgbmVnYXRpdmUgdmFsdWVzIChvciB3b3VsZCBoYXZlIHdyb25nCj4gYmVoYXZp b3VyIHdpdGggbmVnYXRpdmUgdmFsdWVzKSwgdGhlIGFzc2VydCgpIGlzIHBlcmhhcHMganVzdGlm aWVkLCBhcwo+IGl0IGNvdWxkIHByZXZlbnQgZnJvbSBkb2luZyBtb3JlIGhhcm0uCgpJJ2xsIGdv IGFuZCBhdWRpdCBhbGwgb2YgdGhlbS4KClRoYW5rcyBmb3IgdGhlIHJldmlldyEKClBoaWwuCgo+ PiAtLS0KPj4gIGNoYXJkZXYvY2hhci5jIHwgNSArKysrLQo+PiAgMSBmaWxlIGNoYW5nZWQsIDQg aW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQo+Pgo+PiBkaWZmIC0tZ2l0IGEvY2hhcmRldi9j aGFyLmMgYi9jaGFyZGV2L2NoYXIuYwo+PiBpbmRleCBmNmQ2MWZhNWY4Li43MWVjZDMyYjI1IDEw MDY0NAo+PiAtLS0gYS9jaGFyZGV2L2NoYXIuYwo+PiArKysgYi9jaGFyZGV2L2NoYXIuYwo+PiBA QCAtMTU5LDEyICsxNTksMTUgQEAgaW50IHFlbXVfY2hyX3dyaXRlKENoYXJkZXYgKnMsIGNvbnN0 IHVpbnQ4X3QgKmJ1ZiwgaW50IGxlbiwgYm9vbCB3cml0ZV9hbGwpCj4+ICBpbnQgcWVtdV9jaHJf YmVfY2FuX3dyaXRlKENoYXJkZXYgKnMpCj4+ICB7Cj4+ICAgICAgQ2hhckJhY2tlbmQgKmJlID0g cy0+YmU7Cj4+ICsgICAgaW50IHJlY2VpdmFibGVfYnl0ZXM7Cj4+Cj4+ICAgICAgaWYgKCFiZSB8 fCAhYmUtPmNocl9jYW5fcmVhZCkgewo+PiAgICAgICAgICByZXR1cm4gMDsKPj4gICAgICB9Cj4+ Cj4+IC0gICAgcmV0dXJuIGJlLT5jaHJfY2FuX3JlYWQoYmUtPm9wYXF1ZSk7Cj4+ICsgICAgcmVj ZWl2YWJsZV9ieXRlcyA9IGJlLT5jaHJfY2FuX3JlYWQoYmUtPm9wYXF1ZSk7Cj4+ICsgICAgYXNz ZXJ0KHJlY2VpdmFibGVfYnl0ZXMgPj0gMCk7Cj4+ICsgICAgcmV0dXJuIHJlY2VpdmFibGVfYnl0 ZXM7Cj4+ICB9Cj4+Cj4+ICB2b2lkIHFlbXVfY2hyX2JlX3dyaXRlX2ltcGwoQ2hhcmRldiAqcywg dWludDhfdCAqYnVmLCBpbnQgbGVuKQo+PiAtLQo+PiAyLjIwLjEKPj4KCl9fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fClhlbi1kZXZlbCBtYWlsaW5nIGxpc3QK WGVuLWRldmVsQGxpc3RzLnhlbnByb2plY3Qub3JnCmh0dHBzOi8vbGlzdHMueGVucHJvamVjdC5v cmcvbWFpbG1hbi9saXN0aW5mby94ZW4tZGV2ZWw=