[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v23 04/32] change opt->name and opt->str from (c
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v23 04/32] change opt->name and opt->str from (const char *) to (char *) |
Date: |
Tue, 25 Mar 2014 13:23:32 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 |
On 03/21/2014 04:12 AM, Chunyan Liu wrote:
Your subject says "what", but your commit message lacks a "why".
Without a good reason, it's hard to see what this patch is good for.
May I suggest:
qemu_opt_del() already assumes that all QemuOpt instances contain
malloc'd name and value; but it had to cast away const because
opts_start_struct() was doing its own thing and using static storage
instead. By using the correct type and malloced strings everywhere, the
usage of this struct becomes clearer.
> Signed-off-by: Chunyan Liu <address@hidden>
> ---
> include/qemu/option_int.h | 4 ++--
> qapi/opts-visitor.c | 10 +++++++---
> util/qemu-option.c | 4 ++--
> 3 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/include/qemu/option_int.h b/include/qemu/option_int.h
> index 8212fa4..db9ed91 100644
> --- a/include/qemu/option_int.h
> +++ b/include/qemu/option_int.h
> @@ -30,8 +30,8 @@
> #include "qemu/error-report.h"
>
> struct QemuOpt {
> - const char *name;
> - const char *str;
> + char *name;
> + char *str;
While touching this, is it worth trimming the extra spacing before '*'?
It's not like you are preserving alignment or anything. But as that's
cosmetic, and as the code itself is correct, I'm fine if you add this
when you expand your commit message:
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v23 06/32] add qemu_opt_get_*_del functions for replace work, (continued)
- [Qemu-devel] [PATCH v23 06/32] add qemu_opt_get_*_del functions for replace work, Chunyan Liu, 2014/03/21
- [Qemu-devel] [PATCH v23 05/32] move qemu_opt_del ahead for later calling, Chunyan Liu, 2014/03/21
- [Qemu-devel] [PATCH v23 07/32] add qemu_opts_print_help to replace print_option_help, Chunyan Liu, 2014/03/21
- [Qemu-devel] [PATCH v23 04/32] change opt->name and opt->str from (const char *) to (char *), Chunyan Liu, 2014/03/21
- [Qemu-devel] [PATCH v23 08/32] add convert functions between QEMUOptionParameter to QemuOpts, Chunyan Liu, 2014/03/21
- [Qemu-devel] [PATCH v23 10/32] check NULL input for qemu_opts_del, Chunyan Liu, 2014/03/21
- [Qemu-devel] [PATCH v23 11/32] qemu_opts_print: change fprintf stderr to printf, Chunyan Liu, 2014/03/21
- [Qemu-devel] [PATCH v23 12/32] qcow2.c: remove 'assigned' check in amend, Chunyan Liu, 2014/03/21