From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47289) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ScIAw-0001L6-I9 for qemu-devel@nongnu.org; Wed, 06 Jun 2012 11:29:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ScIAq-0005YP-9J for qemu-devel@nongnu.org; Wed, 06 Jun 2012 11:29:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11049) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ScIAq-0005YI-1X for qemu-devel@nongnu.org; Wed, 06 Jun 2012 11:29:28 -0400 Message-ID: <4FCF777B.7000806@redhat.com> Date: Wed, 06 Jun 2012 17:30:03 +0200 From: Laszlo Ersek MIME-Version: 1.0 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> In-Reply-To: <20120606151640.GA7733@illuin> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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: Michael Roth Cc: Paolo Bonzini , =?ISO-8859-1?Q?Andreas_F=E4rbe?= =?ISO-8859-1?Q?r?= , qemu-devel@nongnu.org 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 Laszlo