[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 15/16] convert net_init_bridge() to NetClientOpt
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH 15/16] convert net_init_bridge() to NetClientOptions |
Date: |
Wed, 06 Jun 2012 14:16:02 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:10.0.4) Gecko/20120422 Thunderbird/10.0.4 |
On 06/05/12 23:05, Paolo Bonzini wrote:
> Il 22/05/2012 12:45, Laszlo Ersek ha scritto:
>> Signed-off-by: Laszlo Ersek <address@hidden>
>> ---
>> net/tap.c | 23 ++++++++++++-----------
>> 1 files changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/net/tap.c b/net/tap.c
>> index 7501eba..fdaab2b 100644
>> --- a/net/tap.c
>> +++ b/net/tap.c
>> @@ -512,21 +512,22 @@ static int net_bridge_run_helper(const char *helper,
>> const char *bridge)
>> return -1;
>> }
>>
>> -int net_init_bridge(QemuOpts *opts, const NetClientOptions *new_opts,
>> +int net_init_bridge(QemuOpts *old_opts, const NetClientOptions *opts,
>> const char *name, VLANState *vlan)
>> {
>> + const NetdevBridgeOptions *bridge;
>> + const char *helper, *br;
>> +
>> TAPState *s;
>> int fd, vnet_hdr;
>>
>> - if (!qemu_opt_get(opts, "br")) {
>> - qemu_opt_set(opts, "br", DEFAULT_BRIDGE_INTERFACE);
>> - }
>> - if (!qemu_opt_get(opts, "helper")) {
>> - qemu_opt_set(opts, "helper", DEFAULT_BRIDGE_HELPER);
>> - }
>> + assert(opts->kind == NET_CLIENT_OPTIONS_KIND_BRIDGE);
>> + bridge = opts->bridge;
>> +
>> + helper = bridge->has_helper ? bridge->helper : DEFAULT_BRIDGE_HELPER;
>> + br = bridge->has_br ? bridge->br : DEFAULT_BRIDGE_INTERFACE;
>
> Don't hate me for this, but why not do the same for strdup calls?
>
> foo = bar->has_foo ? g_strdup(bar->foo) : NULL;
>
> earlier in the series?
(I think you mean net_init_nic() in [PATCH 09/16] "convert
net_init_nic() to NetClientOptions".)
I didn't deliberately avoid the conditional operator there. The
net_init_nic() structure seemed OK; it sets all such pointers to
all-bits-zero in a batch (memset()) and only changes those whose
corresponding optarg (QemuOpt) is set. It seemed natural and didn't
summon ?: to my mind.
net_init_bridge() OTOH sets the defaults too on a case-by-case basis,
and it was screaming after ?:.
... No hate whatsoever :)
Laszlo