From: Eric Blake <eblake@redhat.com>
To: Chunyan Liu <cyliu@suse.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
Dong Xu Wang <wdongxu@linux.vnet.ibm.com>,
qemu-devel@nongnu.org, stefanha@redhat.com
Subject: Re: [Qemu-devel] [PATCH v22 05/25] add some QemuOpts functions for replace work
Date: Wed, 12 Mar 2014 06:40:33 -0600 [thread overview]
Message-ID: <532055C1.10100@redhat.com> (raw)
In-Reply-To: <CAERYnoaHWLvRaXvwJQh12FwFuiR07k9HDQgdsTnqz7y6sAwYJg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1501 bytes --]
On 03/11/2014 09:10 PM, Chunyan Liu wrote:
>>>>
>>> Could be if changing qemu_opt_get return value type, but just as said
>>> before,
>>> that will affect many codes.
>>
>> Also, changing an existing function that returns 'const char *' into now
>> returning 'char *' will NOT break any callers if the semantics remain
>> unchanged (what WILL break is if the semantics change where the callers
>> must now free the result)
>
>
> Yes, that's the only change, we need to free the result. There are more
> than 200
> places using qemu_opt_get in existing code, we need to checkout the related
> files
> and free the result one by one.
You can still write your helper function so that if 'del' is true, you
strdup, if 'del' is false, you return the in-place memory. You'll have
to cast away const in the helper, but qemu_opt_get can then restore the
const and you don't have to adjust any existing callers, while still
sharing the underlying implementation rather than duplicating code. But
I guess I'll wait for v23 to see what you actually do in response to my
suggestions. If nothing else, please document design decisions in your
commit message (for example, if you choose to duplicate code rather than
share code in a common helper, explain that the duplication is because
one function returns malloc'd memory while the other returns in-place
memory).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
next prev parent reply other threads:[~2014-03-12 12:40 UTC|newest]
Thread overview: 105+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-10 7:31 [Qemu-devel] [PATCH v22 00/25] replace QEMUOptionParameter with QemuOpts Chunyan Liu
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 01/25] add def_value_str to QemuOptDesc Chunyan Liu
2014-03-10 19:52 ` Eric Blake
2014-03-11 13:29 ` Stefan Hajnoczi
2014-03-12 2:45 ` Chunyan Liu
2014-03-12 8:27 ` Stefan Hajnoczi
2014-03-13 2:46 ` Chunyan Liu
2014-03-13 12:13 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 02/25] qapi: output def_value_str when query command line options Chunyan Liu
2014-03-10 19:57 ` Eric Blake
2014-03-11 6:14 ` Hu Tao
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 03/25] improve some functions in qemu-option.c Chunyan Liu
2014-03-10 20:29 ` Eric Blake
2014-03-10 21:21 ` Eric Blake
2014-03-11 7:26 ` Chunyan Liu
2014-03-11 21:00 ` Leandro Dorileo
2014-03-16 21:19 ` Leandro Dorileo
2014-03-18 7:41 ` Chunyan Liu
2014-03-12 6:49 ` Chunyan Liu
2014-03-17 19:58 ` Leandro Dorileo
2014-03-18 7:49 ` Chunyan Liu
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 04/25] improve assertion in qemu_opt_get functions Chunyan Liu
2014-03-10 21:44 ` Eric Blake
2014-03-12 6:34 ` Chunyan Liu
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 05/25] add some QemuOpts functions for replace work Chunyan Liu
2014-03-10 23:28 ` Eric Blake
2014-03-11 5:29 ` Chunyan Liu
2014-03-11 11:59 ` Eric Blake
2014-03-12 3:10 ` Chunyan Liu
2014-03-12 12:40 ` Eric Blake [this message]
2014-03-13 5:16 ` Chunyan Liu
2014-03-18 5:34 ` Chunyan Liu
2014-03-17 19:35 ` Leandro Dorileo
2014-03-18 3:03 ` Chunyan Liu
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 06/25] add convert functions between QEMUOptionParameter to QemuOpts Chunyan Liu
2014-03-11 4:46 ` Eric Blake
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 07/25] change block layer to support both QemuOpts and QEMUOptionParamter Chunyan Liu
2014-03-11 4:34 ` Eric Blake
2014-03-11 16:54 ` Eric Blake
2014-03-12 6:26 ` Chunyan Liu
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 08/25] cow.c: replace QEMUOptionParameter with QemuOpts Chunyan Liu
2014-03-11 14:12 ` Stefan Hajnoczi
2014-03-11 15:28 ` Eric Blake
2014-03-20 6:56 ` Chunyan Liu
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 09/25] gluster.c: " Chunyan Liu
2014-03-11 14:15 ` Stefan Hajnoczi
2014-03-11 16:58 ` Eric Blake
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 10/25] iscsi.c: " Chunyan Liu
2014-03-11 14:17 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 11/25] qcow.c: " Chunyan Liu
2014-03-11 14:18 ` Stefan Hajnoczi
2014-03-11 17:05 ` Eric Blake
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 12/25] qcow2.c: " Chunyan Liu
2014-03-11 14:21 ` Stefan Hajnoczi
2014-03-11 14:22 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 13/25] qed.c: " Chunyan Liu
2014-03-11 14:24 ` Stefan Hajnoczi
2014-03-20 9:08 ` Chun Yan Liu
2014-03-20 14:14 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 14/25] raw-posix.c: " Chunyan Liu
2014-03-11 14:25 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 15/25] raw-win32.c: " Chunyan Liu
2014-03-11 14:41 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 16/25] raw_bsd.c: " Chunyan Liu
2014-03-11 14:44 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 17/25] rbd.c: " Chunyan Liu
2014-03-11 14:46 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 18/25] sheepdog.c: " Chunyan Liu
2014-03-11 16:01 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 19/25] ssh.c: " Chunyan Liu
2014-03-11 16:01 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 20/25] vdi.c: " Chunyan Liu
2014-03-11 17:50 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 21/25] vmdk.c: " Chunyan Liu
2014-03-11 17:51 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 22/25] vpc.c: " Chunyan Liu
2014-03-11 17:55 ` Stefan Hajnoczi
2014-03-10 7:31 ` [Qemu-devel] [PATCH v22 23/25] vhdx.c: " Chunyan Liu
2014-03-11 17:56 ` Stefan Hajnoczi
2014-03-10 7:32 ` [Qemu-devel] [PATCH v22 24/25] vvfat.c: " Chunyan Liu
2014-03-11 17:06 ` Eric Blake
2014-03-11 18:01 ` Stefan Hajnoczi
2014-03-10 7:32 ` [Qemu-devel] [PATCH v22 25/25] cleanup QEMUOptionParameter Chunyan Liu
2014-03-11 14:06 ` Stefan Hajnoczi
2014-03-17 19:29 ` Leandro Dorileo
2014-03-17 19:43 ` Leandro Dorileo
2014-03-10 7:36 ` [Qemu-devel] [PATCH v22 00/25] replace QEMUOptionParameter with QemuOpts Chun Yan Liu
2014-03-10 7:37 ` Chun Yan Liu
2014-03-10 7:37 ` Chun Yan Liu
2014-03-10 7:38 ` Chun Yan Liu
2014-03-10 7:39 ` Chun Yan Liu
2014-03-10 7:39 ` Chun Yan Liu
2014-03-10 20:22 ` Stefan Hajnoczi
2014-03-11 3:07 ` Chunyan Liu
2014-03-10 22:45 ` Eric Blake
2014-03-11 18:03 ` Stefan Hajnoczi
2014-03-21 0:07 ` Leandro Dorileo
2014-03-21 10:09 ` Chunyan Liu
2014-03-21 12:31 ` Leandro Dorileo
2014-03-24 3:02 ` Chunyan Liu
2014-03-24 15:00 ` Leandro Dorileo
2014-03-25 7:15 ` Chunyan Liu
2014-04-03 9:46 ` Chunyan Liu
2014-03-21 10:34 ` Kevin Wolf
2014-03-21 12:21 ` Leandro Dorileo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=532055C1.10100@redhat.com \
--to=eblake@redhat.com \
--cc=cyliu@suse.com \
--cc=kwolf@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=wdongxu@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).