[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/9] Add API to manipulate dump_bitmap
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 1/9] Add API to manipulate dump_bitmap |
Date: |
Tue, 07 May 2013 10:14:11 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130402 Thunderbird/17.0.5 |
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.
>
> Signed-off-by: Qiao Nuohan <address@hidden>
> Reviewed-by: Zhang Xiaohe <address@hidden>
> ---
> + db->file_name = (char *)g_malloc(strlen(filename) + strlen(tmpname) + 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 <address@hidden>
> + *
> + * 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 <stdio.h> instead of reinventing a new name
for this constant?
> +#define BITPERBYTE (8)
Why not use CHAR_BIT from <limits.h> 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 dump
> bitmap */
> + int no_block; /* number of block cached in buf */
Trailing whitespace. Run your patch series through scripts/checkpatch.pl.
The name no_block sounds like there aren't any blocks. You probably
want the name num_block instead.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH 4/9] Add API to create header of vmcore, (continued)
- [Qemu-devel] [PATCH 4/9] Add API to create header of vmcore, Qiao Nuohan, 2013/05/07
- [Qemu-devel] [PATCH 6/9] Add API to create page, Qiao Nuohan, 2013/05/07
- [Qemu-devel] [PATCH 8/9] Add API to write header, bitmap and page into vmcore, Qiao Nuohan, 2013/05/07
- [Qemu-devel] [PATCH 9/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format, Qiao Nuohan, 2013/05/07
- [Qemu-devel] [PATCH 7/9] Add API to free buf used by creating header, bitmap and page, Qiao Nuohan, 2013/05/07
- [Qemu-devel] [PATCH 3/9] Move include and struct definition to dump.h, Qiao Nuohan, 2013/05/07
- [Qemu-devel] [PATCH 2/9] Add API to manipulate cache_data, Qiao Nuohan, 2013/05/07
- [Qemu-devel] [PATCH 1/9] Add API to manipulate dump_bitmap, Qiao Nuohan, 2013/05/07
- Re: [Qemu-devel] [PATCH 1/9] Add API to manipulate dump_bitmap,
Eric Blake <=
- [Qemu-devel] [PATCH 5/9] Add API to create data of dump bitmap, Qiao Nuohan, 2013/05/07
- Re: [Qemu-devel] [PATCH 0/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format, HATAYAMA Daisuke, 2013/05/08