From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50333) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XwUr8-0001lh-VQ for qemu-devel@nongnu.org; Thu, 04 Dec 2014 06:46:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XwUr3-0006O5-GF for qemu-devel@nongnu.org; Thu, 04 Dec 2014 06:45:58 -0500 Date: Thu, 4 Dec 2014 19:45:36 +0800 From: Fam Zheng Message-ID: <20141204114536.GA27879@ad.nay.redhat.com> References: <1417685215-5289-1-git-send-email-armbru@redhat.com> <20141204093414.GB28961@ad.nay.redhat.com> <87mw73snpo.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87mw73snpo.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-trivial@nongnu.org, qemu-devel@nongnu.org On Thu, 12/04 11:39, Markus Armbruster wrote: > Fam Zheng writes: > > > On Thu, 12/04 10:26, Markus Armbruster wrote: > >> Signed-off-by: Markus Armbruster > >> --- > >> backends/rng-random.c | 6 +----- > >> hw/tpm/tpm_passthrough.c | 4 +--- > >> util/uri.c | 43 +++++++++++++++++-------------------------- > >> 3 files changed, 19 insertions(+), 34 deletions(-) > >> > >> diff --git a/backends/rng-random.c b/backends/rng-random.c > >> index 601d9dc..4f85a8e 100644 > >> --- a/backends/rng-random.c > >> +++ b/backends/rng-random.c > >> @@ -88,11 +88,7 @@ static char *rng_random_get_filename(Object *obj, Error **errp) > >> { > >> RndRandom *s = RNG_RANDOM(obj); > >> > >> - if (s->filename) { > >> - return g_strdup(s->filename); > >> - } > >> - > >> - return NULL; > >> + return g_strdup(s->filename); > >> } > >> > >> static void rng_random_set_filename(Object *obj, const char *filename, > >> diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c > >> index 56e9e0f..2bf3c6f 100644 > >> --- a/hw/tpm/tpm_passthrough.c > >> +++ b/hw/tpm/tpm_passthrough.c > >> @@ -400,9 +400,7 @@ static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb) > >> const char *value; > >> > >> value = qemu_opt_get(opts, "cancel-path"); > >> - if (value) { > >> - tb->cancel_path = g_strdup(value); > >> - } > >> + tb->cancel_path = g_strdup(value); > >> > >> value = qemu_opt_get(opts, "path"); > >> if (!value) { > >> diff --git a/util/uri.c b/util/uri.c > >> index e348c17..bbf2832 100644 > >> --- a/util/uri.c > >> +++ b/util/uri.c > >> @@ -1736,24 +1736,21 @@ uri_resolve(const char *uri, const char *base) { > >> goto done; > >> if ((ref->scheme == NULL) && (ref->path == NULL) && > >> ((ref->authority == NULL) && (ref->server == NULL))) { > >> - if (bas->scheme != NULL) > >> - res->scheme = g_strdup(bas->scheme); > >> + res->scheme = g_strdup(bas->scheme); > >> if (bas->authority != NULL) > >> res->authority = g_strdup(bas->authority); > >> else if (bas->server != NULL) { > >> - res->server = g_strdup(bas->server); > >> - if (bas->user != NULL) > >> - res->user = g_strdup(bas->user); > >> - res->port = bas->port; > >> + res->server = g_strdup(bas->server); > >> + res->user = g_strdup(bas->user); > >> + res->port = bas->port; You fixed indentation for res->server and res->port ... > >> } > >> - if (bas->path != NULL) > >> - res->path = g_strdup(bas->path); > >> - if (ref->query != NULL) > >> + res->path = g_strdup(bas->path); > >> + if (ref->query != NULL) { > >> res->query = g_strdup (ref->query); ... but not for res->query. > >> + } else { > >> + res->query = g_strdup(bas->query); > >> + } > >> + res->fragment = g_strdup(ref->fragment); > >> goto step_7; > >> } > >> > >> @@ -1767,13 +1764,10 @@ uri_resolve(const char *uri, const char *base) { > >> val = uri_to_string(ref); > >> goto done; > >> } > >> - if (bas->scheme != NULL) > >> - res->scheme = g_strdup(bas->scheme); > >> + res->scheme = g_strdup(bas->scheme); > >> > >> - if (ref->query != NULL) > >> - res->query = g_strdup(ref->query); > >> - if (ref->fragment != NULL) > >> - res->fragment = g_strdup(ref->fragment); > >> + res->query = g_strdup(ref->query); > >> + res->fragment = g_strdup(ref->fragment); > >> > >> /* > >> * 4) If the authority component is defined, then the reference is a > >> @@ -1787,20 +1781,17 @@ uri_resolve(const char *uri, const char *base) { > >> res->authority = g_strdup(ref->authority); > >> else { > >> res->server = g_strdup(ref->server); > >> - if (ref->user != NULL) > >> - res->user = g_strdup(ref->user); > >> + res->user = g_strdup(ref->user); > >> res->port = ref->port; > >> } > >> - if (ref->path != NULL) > >> - res->path = g_strdup(ref->path); > >> + res->path = g_strdup(ref->path); > >> goto step_7; > >> } > >> if (bas->authority != NULL) > >> res->authority = g_strdup(bas->authority); > >> else if (bas->server != NULL) { > >> - res->server = g_strdup(bas->server); > >> - if (bas->user != NULL) > >> - res->user = g_strdup(bas->user); > >> + res->server = g_strdup(bas->server); > >> + res->user = g_strdup(bas->user); > >> res->port = bas->port; and not for res->port. > >> } > >> > >> -- > >> 1.9.3 > >> > >> > > > > Very confusing tab/whitespace mixture. Code is better, format is worse. I'm not > > sure it's a win. :) > > As per standard operating procedure, I expanded tabs in the lines I > touched. No visual difference, except in patches. > > What do you want me to do? > > 1. Don't expand tabs, ignore checkpatch.pl whining > > 2. Expand tabs in touched lines (current patch) > > 3. Expand all tabs in uri_resolve() (in a separate patch, of course) > > 4. Expand all tabs in util/uri.c (in a separate patch, of course) > Well, I never know what to do with legacy tabs, perhaps it's not worth polluting git blame. The code looks good, as a reviewer I'm happy to add my Reviewed-by: Fam Zheng if maintainers are happy with the indentation. Fam