From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34392) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yi1qX-0000Me-Mp for qemu-devel@nongnu.org; Tue, 14 Apr 2015 10:29:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yi1qS-0008KG-Ex for qemu-devel@nongnu.org; Tue, 14 Apr 2015 10:29:49 -0400 Received: from mail-wg0-x22d.google.com ([2a00:1450:400c:c00::22d]:33695) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yi1qS-0008K6-8g for qemu-devel@nongnu.org; Tue, 14 Apr 2015 10:29:44 -0400 Received: by wgin8 with SMTP id n8so14089765wgi.0 for ; Tue, 14 Apr 2015 07:29:43 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <552D2451.5040403@redhat.com> Date: Tue, 14 Apr 2015 16:29:37 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1428523369-13700-1-git-send-email-peter.maydell@linaro.org> <552D1940.5030709@redhat.com> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] qga/commands-posix.c: Use correct types with g_base64_decode() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Michael Roth , QEMU Developers , Patch Tracking On 14/04/2015 15:45, Peter Maydell wrote: > On 14 April 2015 at 14:42, Paolo Bonzini wrote: >> >> >> On 08/04/2015 22:02, Peter Maydell wrote: >>> The second argument of g_base64_decode() is a 'gsize *', not a >>> 'size_t *'. Some compilation environments (like building 32-bit PPC >>> binaries on a PPC64 system) will complain about the mismatch: >>> >>> CC qga/commands-posix.o >>> qga/commands-posix.c: In function 'qmp_guest_set_user_password': >>> qga/commands-posix.c:1908:5: error: passing argument 2 of 'g_base64_decode' from incompatible pointer type [-Werror] >>> In file included from /usr/include/glib-2.0/glib.h:37:0, >>> from qga/commands-posix.c:14: >>> /usr/include/glib-2.0/glib/gbase64.h:49:9: note: expected ‘gsize *’ but argument is of type ‘size_t *’ >>> >>> (We previously fixed errors of this type in commit 3d1bba20.) >>> >>> Signed-off-by: Peter Maydell >> >> I think this patch is wrong. Considering what Thomas was doing >> ("playing around with --extra-cflags=-m32" on x86) this looks like >> you're using the 64-bit glib headers while doing a 32-bit compilation. > > I sort of agree, but I don't think the patch is wrong as such. > The API of the function we're calling demands a pointer to a > 'gsize', so we should do that, not rely implicitly on 'gsize' > and 'size_t' being interchangeable. But you can: gsize is defined to be "An unsigned integer type of the result of the sizeof operator, corresponding to the size_t type defined in C99. This type is wide enough to hold the numeric value of a pointer, so it is usually 32bit wide on a 32bit platform and 64bit wide on a 64bit platform". So it's impossible to disagree---the patch is correct in the sense that it fixes a warning. But it is not correct as far as its original rationale (compiling with -m32) goes. What you'll get is a library compiled for 32-bit gsize, receving arguments from a program that passes 64-bit gsize. That's an ABI mismatch, and one which is unlikely to work on most 32-bit architectures; in this sense the patch is wrong. If anything, I would add a QEMU_BUILD_BUG_ON(sizeof(gsize) != sizeof(size_t)) to catch the problem, since we've had many experienced developers caught unprepared. At which point we've added another safety net and the patch becomes 101% correct---but you get a compilation error anyway, so the original purpose of the patch is not fulfilled. All in all, I don't think this patch is 2.3 material. Paolo > (I have no idea why glib sees fit to define its own versions > of the standard types, but given that it does we should get > the interfacing to them right...) That's 1995-vintage portability for you. :( Paolo