From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45171) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ScIck-00075A-2e for qemu-devel@nongnu.org; Wed, 06 Jun 2012 11:58:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ScIcd-0003Sb-ES for qemu-devel@nongnu.org; Wed, 06 Jun 2012 11:58:17 -0400 Received: from mail-pz0-f45.google.com ([209.85.210.45]:53069) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ScIcd-0003S4-1T for qemu-devel@nongnu.org; Wed, 06 Jun 2012 11:58:11 -0400 Received: by dadv2 with SMTP id v2so9742546dad.4 for ; Wed, 06 Jun 2012 08:58:08 -0700 (PDT) Sender: fluxion Date: Wed, 6 Jun 2012 10:58:01 -0500 From: Michael Roth Message-ID: <20120606155801.GB7733@illuin> References: <1337683555-13301-1-git-send-email-lersek@redhat.com> <4FCE7684.2070206@redhat.com> <4FCF5512.9000704@redhat.com> <4FCF5BA8.3010201@suse.de> <4FCF64E4.50701@redhat.com> <20120606151640.GA7733@illuin> <4FCF777B.7000806@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4FCF777B.7000806@redhat.com> Subject: Re: [Qemu-devel] [PATCH 00/16] introduce OptsVisitor, rebase -net/-netdev parsing List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek Cc: Paolo Bonzini , Andreas =?iso-8859-1?Q?F=E4rber?= , qemu-devel@nongnu.org On Wed, Jun 06, 2012 at 05:30:03PM +0200, Laszlo Ersek wrote: > On 06/06/12 17:16, Michael Roth wrote: > > On Wed, Jun 06, 2012 at 04:10:44PM +0200, Paolo Bonzini wrote: > > >> The uintXX visitors do not fail if you pass a negative value. I'm fine > >> with including the patch with the small bug and fixing it as a > >> follow-up, there's plenty of time before 1.2. > > > > How would we implement such a check? > > > > In the case of uint64_t, the field we're visiting is passed in as a > > uint64_t*, so -1 is indistinguishable from the unsigned interpretation > > of the field, which is within the valid range. > > > > For uintXX_t where XX < 64, a negative value would exceed the UINTXX_MAX > > check, so those cases are already handled. > > > > Or am I missing something? > > I found three instances of the patch on the list: > > http://lists.nongnu.org/archive/html/qemu-devel/2012-04/msg00333.html > http://lists.nongnu.org/archive/html/qemu-devel/2012-04/msg01292.html > http://lists.nongnu.org/archive/html/qemu-devel/2012-04/msg04068.html > > looking at the third one, all of > > - visit_type_uint8() > - visit_type_uint16() > - visit_type_uint32() > - visit_type_uint64() > > seem to define "value" as an int64_t. Thus when we fall back to > (*v->type_int)(), the comparison is still done against an int64_t. Since > "int" is equivalent to "int32_t" on the platforms I can think of, and > "int64_t" to "long long", the comparisons are evaluated as follows: > > value > UINT8_MAX > value > UINT16_MAX > > First the right hand sides are promoted to "int" (with unchanged value), > and then "int" is converted to "long long" (both signed, different > conversion rank). > > value > UINT32_MAX > > The right hand side is directly converted to "long long" (signed vs. > unsigned, signed has greater rank and can represent all values of the > lower-rank unsigned type). > > I propose > > value < 0 || value > UINT8_MAX > value < 0 || value > UINT16_MAX > value < 0 || value > UINT32_MAX > value < 0 Thanks, this does indeed seem warranted. I'm fine either of the options Andreas' suggested (a fix-up to squash into into qom-next version or a seperate patch to apply to qom-next). Only thing I'd like to avoid is having a modified/squashed patch floating around. > > Laszlo >