[Qemu-devel] [PATCH] vl.c: Fix Coverity complaining for vmstate_dump_file

[Qemu-devel] [PATCH] vl.c: Fix Coverity complaining for vmstate_dump_file

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

commit abfd9ce3(migration: dump vmstate info as a json
file for static analysis) introduce a new command,
'-dump-vmstate', that takes a filename
as an argument.  When executed, QEMU will dump the vmstate information
for the machine type it's invoked with to the file, and quit.

Apparently, it supports one '-dump-vmstate' option,
otherwise, the vmstate_dump_file will be overwritten.

Of course, the resource will be freed when Qemu quit, but The code logic
is not good, it will make Coverity complaining.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 vl.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/vl.c b/vl.c
index f6b3546..5ef3af9 100644
--- a/vl.c
+++ b/vl.c
@@ -3746,6 +3746,11 @@ int main(int argc, char **argv, char **envp)
                 configure_msg(opts);
                 break;
             case QEMU_OPTION_dump_vmstate:
+                if (vmstate_dump_file) {
+                    fprintf(stderr, "qemu: only one '-dump-vmstate' "
+                            "option may be given\n");
+                    exit(1);
+                }
                 vmstate_dump_file = fopen(optarg, "w");
                 if (vmstate_dump_file == NULL) {
                     fprintf(stderr, "open %s: %s\n", optarg, strerror(errno));
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH] vl.c: Fix Coverity complaining for vmstate_dump_file

[Amit Shah]

On (Wed) 29 Oct 2014 [20:49:43], arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> commit abfd9ce3(migration: dump vmstate info as a json
> file for static analysis) introduce a new command,
> '-dump-vmstate', that takes a filename
> as an argument.  When executed, QEMU will dump the vmstate information
> for the machine type it's invoked with to the file, and quit.
> 
> Apparently, it supports one '-dump-vmstate' option,
> otherwise, the vmstate_dump_file will be overwritten.

Suggested re-wording of this para:

However, only one instance of the -dump-vmstate option is supported.
If more were given, the vmstate_dump_file could be overwritten.

> Of course, the resource will be freed when Qemu quit, but The code logic
> is not good, it will make Coverity complaining.

Suggested re-wording:

This fix also helps silence a Coverity error.

> Signed-off-by: Gonglei <arei.gonglei@huawei.com>

Reviewed-by: Amit Shah <amit.shah@redhat.com>

Thanks,

		Amit

Re: [Qemu-devel] [PATCH] vl.c: Fix Coverity complaining for vmstate_dump_file

[Paolo Bonzini]

On 10/29/2014 01:49 PM, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> commit abfd9ce3(migration: dump vmstate info as a json
> file for static analysis) introduce a new command,
> '-dump-vmstate', that takes a filename
> as an argument.  When executed, QEMU will dump the vmstate information
> for the machine type it's invoked with to the file, and quit.
> 
> Apparently, it supports one '-dump-vmstate' option,
> otherwise, the vmstate_dump_file will be overwritten.
> 
> Of course, the resource will be freed when Qemu quit, but The code logic
> is not good, it will make Coverity complaining.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  vl.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/vl.c b/vl.c
> index f6b3546..5ef3af9 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3746,6 +3746,11 @@ int main(int argc, char **argv, char **envp)
>                  configure_msg(opts);
>                  break;
>              case QEMU_OPTION_dump_vmstate:
> +                if (vmstate_dump_file) {
> +                    fprintf(stderr, "qemu: only one '-dump-vmstate' "
> +                            "option may be given\n");
> +                    exit(1);
> +                }
>                  vmstate_dump_file = fopen(optarg, "w");
>                  if (vmstate_dump_file == NULL) {
>                      fprintf(stderr, "open %s: %s\n", optarg, strerror(errno));
> 

Applied, thanks.

Paolo