From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:49314)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <flukshun@gmail.com>) id 1UaR0V-0001lG-Ad
	for qemu-devel@nongnu.org; Thu, 09 May 2013 09:35:40 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <flukshun@gmail.com>) id 1UaR0U-00069a-45
	for qemu-devel@nongnu.org; Thu, 09 May 2013 09:35:39 -0400
Received: from mail-oa0-f50.google.com ([209.85.219.50]:39526)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <flukshun@gmail.com>) id 1UaR0T-00069S-V9
	for qemu-devel@nongnu.org; Thu, 09 May 2013 09:35:38 -0400
Received: by mail-oa0-f50.google.com with SMTP id l20so427948oag.9
	for <qemu-devel@nongnu.org>; Thu, 09 May 2013 06:35:37 -0700 (PDT)
Sender: fluxion <flukshun@gmail.com>
Date: Thu, 9 May 2013 08:32:57 -0500
From: mdroth <mdroth@linux.vnet.ibm.com>
Message-ID: <20130509133257.GA13213@vm>
References: <1368056037-16350-1-git-send-email-mdroth@linux.vnet.ibm.com>
	<1368056037-16350-7-git-send-email-mdroth@linux.vnet.ibm.com>
	<518B9707.2050809@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <518B9707.2050809@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 6/8] qapi: add native list coverage for
 visitor serialization tests
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Laszlo Ersek <lersek@redhat.com>
Cc: akong@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com

On Thu, May 09, 2013 at 02:31:03PM +0200, Laszlo Ersek wrote:
> On 05/09/13 01:33, Michael Roth wrote:
> 
> > +        case PTYPE_NUMBER: {
> > +            numberList *ptr;
> > +            char *double1, *double2;
> > +            if (cur_head) {
> > +                ptr = cur_head;
> > +                cur_head = ptr->next;
> > +            } else {
> > +                cur_head = ptr = pl_copy.value.numbers;
> > +            }
> > +            /* we serialize with %f for our reference visitors, so rather than
> > +             * fuzzy * floating math to test "equality", just compare the
> > +             * formatted values
> > +             */
> 
> I think this comment block has been copied from elsewhere in this file,
> indented more deeply and re-filled. There's an asterisk in the comment
> body now.

Yes :) Will fix it on the next pass.

> 
> 
> > +            double1 = g_malloc0(calc_float_string_storage(pt->value.number));
> > +            double2 = g_malloc0(calc_float_string_storage(ptr->value));
> > +            g_assert_cmpstr(double1, ==, double2);
> 
> Are you comparing empty strings? Space is allocated and zeroed, but I
> can't see where the values are actually formatted.

Well, that's embarassing...

> 
> (Same holds for the original instance of this code, test_primitives().)
> 

but at least I can blame it on the fact that I copied it from here
(we'll just ignore who the original author was :)

Will add a patch on the next pass to fix the original instance
beforehand.

Thanks for the catch!

> > +            g_free(double1);
> > +            g_free(double2);
> > +            break;
> > +        }
> 
> Thanks,
> Laszlo
>