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 <qiaonuo...@cn.fujitsu.com> > Reviewed-by: Zhang Xiaohe <zhan...@cn.fujitsu.com> > ---
> + 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 <qiaonuo...@cn.fujitsu.com>
> + *
> + * 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
