From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:49803) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ScF8e-00054I-Pj for qemu-devel@nongnu.org; Wed, 06 Jun 2012 08:15:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ScF8a-00036J-1d for qemu-devel@nongnu.org; Wed, 06 Jun 2012 08:15:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33628) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ScF8Z-00036A-PX for qemu-devel@nongnu.org; Wed, 06 Jun 2012 08:14:55 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q56CEsGh031979 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 6 Jun 2012 08:14:54 -0400 Message-ID: <4FCF4A02.2080508@redhat.com> Date: Wed, 06 Jun 2012 14:16:02 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <1337683555-13301-1-git-send-email-lersek@redhat.com> <1337683555-13301-16-git-send-email-lersek@redhat.com> <4FCE74AA.7030506@redhat.com> In-Reply-To: <4FCE74AA.7030506@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 15/16] convert net_init_bridge() to NetClientOptions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org On 06/05/12 23:05, Paolo Bonzini wrote: > Il 22/05/2012 12:45, Laszlo Ersek ha scritto: >> Signed-off-by: Laszlo Ersek >> --- >> 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