Re: [Qemu-devel] [PATCH 2/3] FVD: Added the simulated 'blksim' driver

[Andreas Färber]

Am 21.01.2011 um 23:19 schrieb Chunqiang Tang:

> diff --git a/block/blksim.c b/block/blksim.c
> new file mode 100644
> index 0000000..a92ba11
> --- /dev/null
> +++ b/block/blksim.c

Some formal comments, since you're introducing a new file:

> @@ -0,0 +1,752 @@
> +/*

Headers usually start with a one-line summary, "QEMU simulated block  
driver" maybe?

> + * Copyright (c) 2010-2011 IBM
> + *
> + * Authors:
> + *         Chunqiang Tang <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.
> + * See the COPYING file in the top-level directory.

Can you make this GPLv2-or-later to avoid future hassles?

> +#ifndef TRUE
> +# define TRUE 1
> +#endif
> +
> +#ifndef FALSE
> +# define FALSE 0
> +#endif

I don't think these two belong here.

stdbool.h defines true and false with identical values.
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/stdbool.h.html

Not sure about TRUE and FALSE.
If we want them as local definitions, they should rather go to qemu- 
common.h than to individual source files.

Regards,
Andreas