From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42526) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W9EpH-0006Qm-Lt for qemu-devel@nongnu.org; Fri, 31 Jan 2014 09:12:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W9EpB-0004bM-HG for qemu-devel@nongnu.org; Fri, 31 Jan 2014 09:12:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:65204) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W9EpB-0004az-94 for qemu-devel@nongnu.org; Fri, 31 Jan 2014 09:12:05 -0500 Message-ID: <52EBAF1C.8050006@redhat.com> Date: Fri, 31 Jan 2014 15:11:40 +0100 From: Laszlo Ersek MIME-Version: 1.0 References: <52EA89D0.8030006@redhat.com> <1391175905-61306-1-git-send-email-tumanova@linux.vnet.ibm.com> <1391175905-61306-2-git-send-email-tumanova@linux.vnet.ibm.com> In-Reply-To: <1391175905-61306-2-git-send-email-tumanova@linux.vnet.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2] Define the architecture for compressed dump format. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ekaterina Tumanova Cc: borntraeger@de.ibm.com, qiaonuohan@cn.fujitsu.com, Public KVM Mailing List one not-so-important comment below: On 01/31/14 14:45, Ekaterina Tumanova wrote: > Signed-off-by: Ekaterina Tumanova > --- > dump.c | 7 +++++-- > target-i386/cpu.h | 2 ++ > target-s390x/cpu.h | 1 + > 3 files changed, 8 insertions(+), 2 deletions(-) >=20 > diff --git a/dump.c b/dump.c > index 8f64aab..25503bc 100644 > --- a/dump.c > +++ b/dump.c > @@ -32,6 +32,9 @@ > #ifdef CONFIG_SNAPPY > #include > #endif > +#ifndef ELF_MACHINE_UNAME > +#define ELF_MACHINE_UNAME "Unknown" > +#endif > =20 > static uint16_t cpu_convert_to_target16(uint16_t val, int endian) > { > @@ -817,7 +820,7 @@ static int create_header32(DumpState *s) > dh->nr_cpus =3D cpu_convert_to_target32(s->nr_cpus, endian); > bitmap_blocks =3D DIV_ROUND_UP(s->len_dump_bitmap, block_size) * 2= ; > dh->bitmap_blocks =3D cpu_convert_to_target32(bitmap_blocks, endia= n); > - memcpy(&(dh->utsname.machine), "i686", 4); > + strncpy((char *)&(dh->utsname.machine), ELF_MACHINE_UNAME, sizeof(= dh->utsname.machine)); "dh->utsname.machine" is actually an array of characters, if I recall correctly (see NewUtsname in patch 08). The expression in the first argument takes the address of the entire array (the resultant pointer has type pointer-to-array). I didn't call it out with memcpy(), because it didn't really matter. But now it looks a bit gross, because as 2nd step we convert the pointer-to-array back to pointer-to-char. It would be simpler to write strncpy(dh->utsname.machine, ELF_MACHINE_UNAME, sizeof(dh->utsname.machine)) where "dh->utsname.machine" decays to a pointer to its first element: 6. Language 6.3 Conversions 6.3.2 Other operands 6.3.2.1 Lvalues, arrays, and function designators 3 Except when it is the operand of the sizeof operator or the unary & operator, or is a string literal used to initialize an array, an expression that has type =91=91array of type=92=92 is converted to a= n expression with type =91=91pointer to type=92=92 that points to the initial element of the array object and is not an lvalue. If the array object has register storage class, the behavior is undefined. I should have probably noticed this in v1 of the followup patch. I don't insist on upating it of course. Reviewed-by: Laszlo Ersek Nonetheless, if you want to fix that up in a v3, please keep my R-b. Thanks Laszlo