From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39664) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZkX0-00020H-3U for qemu-devel@nongnu.org; Tue, 07 May 2013 12:14:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UZkWy-0000dh-57 for qemu-devel@nongnu.org; Tue, 07 May 2013 12:14:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:13101) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZkWx-0000dK-O8 for qemu-devel@nongnu.org; Tue, 07 May 2013 12:14:20 -0400 Message-ID: <51892853.8040001@redhat.com> Date: Tue, 07 May 2013 10:14:11 -0600 From: Eric Blake MIME-Version: 1.0 References: <1367911007-13990-1-git-send-email-qiaonuohan@cn.fujitsu.com> <1367911007-13990-2-git-send-email-qiaonuohan@cn.fujitsu.com> In-Reply-To: <1367911007-13990-2-git-send-email-qiaonuohan@cn.fujitsu.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2AWMENSIERAOJKTVVBGDF" Subject: Re: [Qemu-devel] [PATCH 1/9] Add API to manipulate dump_bitmap List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Qiao Nuohan Cc: qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2AWMENSIERAOJKTVVBGDF Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 05/07/2013 01:16 AM, Qiao Nuohan wrote: > Struct dump_bitmap is associated with a tmp file, and the tmp file can = be used > to save data of bitmap in kdump-compressed format temporarily. > The following patch will use these functions to get the data of bitmap = and cache > them into tmp files. >=20 > Signed-off-by: Qiao Nuohan > Reviewed-by: Zhang Xiaohe > --- > + db->file_name =3D (char *)g_malloc(strlen(filename) + strlen(tmpna= me) + 1); > + > + strcpy(db->file_name, tmpname); > + strcat(db->file_name, "/"); > + strcat(db->file_name, filename); Off-by-one buffer overflow, since you forgot space for the NUL byte. We use C, not C++, so you don't need to cast the result of g_malloc(). > +++ b/include/dump_bitmap.h > @@ -0,0 +1,57 @@ > +/* > + * QEMU dump bitmap > + * > + * Copyright Fujitsu, Corp. 2013 > + * > + * Authors: > + * Qiao Nuohan > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or = later. > + * See the COPYING file in the top-level directory. > + * > + */ > + No double-inclusion guard? > +#define TMP_DIR "/tmp" Why not reuse P_tmpdir from instead of reinventing a new name for this constant? > +#define BITPERBYTE (8) Why not use CHAR_BIT from instead of reinventing a new name for this constant? > +#define BUFSIZE_BITMAP (4096) > +#define PFN_BUFBITMAP (BITPERBYTE * BUFSIZE_BITMAP) > + > +struct dump_bitmap { > + int fd; /* fd of the tmp file used to store du= mp bitmap */ > + int no_block; /* number of block cached in buf */=20 Trailing whitespace. Run your patch series through scripts/checkpatch.pl= =2E The name no_block sounds like there aren't any blocks. You probably want the name num_block instead. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2AWMENSIERAOJKTVVBGDF Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJRiShTAAoJEKeha0olJ0NqX7gIAJfFIECxW97z27ZqCcgbIen6 RUioXKmDnpyHf2HASzFSvqh/Ce4JUYDWL82U1xm1hHSLjMFsmyxBEHeuifqkVr60 H7PEotTCtFLmE9YW+T87sj5RmaaNhKuvPjMjl3mD261RmIYfsE693QyktLYSIinm pRje9SgtO/o9ypWNx/FyV5nvl3urCLiedhJSFIM4jTCNwBH2JAP0cY+8A1I8RPgC 4uU7qyAx/gnXcm0lD7Ohtavm6eTcRLWwZ8iZklRo/4OWz+FmeXH6WHJOldrxK6wx xBtwLGLgWLAYRpnJ2usMQBbfLWGKKs2Ciu5gXMoCWVUK1DAe5irGwiO7R2ViOIA= =db+e -----END PGP SIGNATURE----- ------enig2AWMENSIERAOJKTVVBGDF--