From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:52157) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gwPZq-0007iT-Ly for qemu-devel@nongnu.org; Wed, 20 Feb 2019 05:58:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gwPZp-0003fa-RN for qemu-devel@nongnu.org; Wed, 20 Feb 2019 05:58:10 -0500 Date: Wed, 20 Feb 2019 11:57:49 +0100 From: Cornelia Huck Message-ID: <20190220115749.7589f868.cohuck@redhat.com> In-Reply-To: References: <20190220010232.18731-1-philmd@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 00/25] chardev: Convert qemu_chr_write() to take a size_t argument List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?TWFyYy1BbmRyw6k=?= Lureau Cc: Philippe =?UTF-8?B?TWF0aGlldS1EYXVkw6k=?= , 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, 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 Wed, 20 Feb 2019 11:53:42 +0100 Marc-Andr=C3=A9 Lureau wrote: > Hi >=20 > On Wed, Feb 20, 2019 at 2:02 AM Philippe Mathieu-Daud=C3=A9 > wrote: > > > > Hi, > > > > This series convert the chardev::qemu_chr_write() to take unsigned > > length argument. To do so I went through all caller and checked if > > there are no negative value possible. =20 >=20 >=20 > Changing signedness is problematic and can easily introduce bugs that > are easy to miss during review. >=20 > I agree with Cornelia about idiomatic use of int. Changing "int" for > "size_t" isn't systematically a clear win. >=20 > Even Google C++ style recommends to avoid unsigned types "(except for > representing bitfields or modular arithmetic). Do not use an unsigned > type merely to assert that a variable is non-negative." > https://google.github.io/styleguide/cppguide.html#Integer_Types - see rat= ionale >=20 > Since Paolo you suggested the change, could you give some convincing > arguments that it's worth taking the plunge? FWIW, using an explicitly unsigned type for a length sounds fine; but not all conversions are really convincing (albeit not wrong).