* [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup()
@ 2014-12-04 9:26 Markus Armbruster
2014-12-04 9:34 ` Fam Zheng
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Markus Armbruster @ 2014-12-04 9:26 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
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;
}
- 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);
- else if (bas->query != NULL)
- res->query = g_strdup(bas->query);
- if (ref->fragment != NULL)
- res->fragment = g_strdup(ref->fragment);
+ } 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;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup()
2014-12-04 9:26 [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup() Markus Armbruster
@ 2014-12-04 9:34 ` Fam Zheng
2014-12-04 10:39 ` Markus Armbruster
2014-12-04 19:22 ` Eric Blake
2014-12-10 8:31 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
2 siblings, 1 reply; 8+ messages in thread
From: Fam Zheng @ 2014-12-04 9:34 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-trivial, qemu-devel
On Thu, 12/04 10:26, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> 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;
> }
> - 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);
> - else if (bas->query != NULL)
> - res->query = g_strdup(bas->query);
> - if (ref->fragment != NULL)
> - res->fragment = g_strdup(ref->fragment);
> + } 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;
> }
>
> --
> 1.9.3
>
>
Very confusing tab/whitespace mixture. Code is better, format is worse. I'm not
sure it's a win. :)
Fam
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup()
2014-12-04 9:34 ` Fam Zheng
@ 2014-12-04 10:39 ` Markus Armbruster
2014-12-04 11:45 ` Fam Zheng
2014-12-04 19:17 ` Eric Blake
0 siblings, 2 replies; 8+ messages in thread
From: Markus Armbruster @ 2014-12-04 10:39 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-trivial, qemu-devel
Fam Zheng <famz@redhat.com> writes:
> On Thu, 12/04 10:26, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> 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;
>> }
>> - 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);
>> - else if (bas->query != NULL)
>> - res->query = g_strdup(bas->query);
>> - if (ref->fragment != NULL)
>> - res->fragment = g_strdup(ref->fragment);
>> + } 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;
>> }
>>
>> --
>> 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)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup()
2014-12-04 10:39 ` Markus Armbruster
@ 2014-12-04 11:45 ` Fam Zheng
2014-12-04 12:43 ` Markus Armbruster
2014-12-04 19:17 ` Eric Blake
1 sibling, 1 reply; 8+ messages in thread
From: Fam Zheng @ 2014-12-04 11:45 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-trivial, qemu-devel
On Thu, 12/04 11:39, Markus Armbruster wrote:
> Fam Zheng <famz@redhat.com> writes:
>
> > On Thu, 12/04 10:26, Markus Armbruster wrote:
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >> 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 <famz@redhat.com>
if maintainers are happy with the indentation.
Fam
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup()
2014-12-04 11:45 ` Fam Zheng
@ 2014-12-04 12:43 ` Markus Armbruster
0 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2014-12-04 12:43 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-trivial, qemu-devel
Fam Zheng <famz@redhat.com> writes:
> On Thu, 12/04 11:39, Markus Armbruster wrote:
>> Fam Zheng <famz@redhat.com> writes:
>>
>> > On Thu, 12/04 10:26, Markus Armbruster wrote:
>> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> >> ---
>> >> 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 ...
... because I touched these lines...
>> >> }
>> >> - 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.
... because I didn't touch this line.
>> >> + } 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.
Likewise.
>> >> }
>> >>
>> >> --
>> >> 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 <famz@redhat.com>
>
> if maintainers are happy with the indentation.
In my private opinion, we should either not bother expanding tabs at
all, or we should've expanded them wholesale long ago. Not worth
rearguing now.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup()
2014-12-04 10:39 ` Markus Armbruster
2014-12-04 11:45 ` Fam Zheng
@ 2014-12-04 19:17 ` Eric Blake
1 sibling, 0 replies; 8+ messages in thread
From: Eric Blake @ 2014-12-04 19:17 UTC (permalink / raw)
To: Markus Armbruster, Fam Zheng; +Cc: qemu-trivial, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 944 bytes --]
On 12/04/2014 03:39 AM, Markus Armbruster wrote:
> 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)
My preferred choice first: 2, 4, 3, 1
That is, I'm fine with how you did it. If you are going to clean up
tabs as a separate patch, I'd prefer you do it for the whole file rather
than just one function. And I'd rather a tab cleanup than ignoring
checkpatch.pl, but not at the expense of favoring a tab cleanup ahead of
the current proposed patch.
--
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: 539 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup()
2014-12-04 9:26 [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup() Markus Armbruster
2014-12-04 9:34 ` Fam Zheng
@ 2014-12-04 19:22 ` Eric Blake
2014-12-10 8:31 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
2 siblings, 0 replies; 8+ messages in thread
From: Eric Blake @ 2014-12-04 19:22 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: qemu-trivial
[-- Attachment #1: Type: text/plain, Size: 484 bytes --]
On 12/04/2014 02:26 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> backends/rng-random.c | 6 +-----
> hw/tpm/tpm_passthrough.c | 4 +---
> util/uri.c | 43 +++++++++++++++++--------------------------
> 3 files changed, 19 insertions(+), 34 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
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: 539 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] Drop superfluous conditionals around g_strdup()
2014-12-04 9:26 [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup() Markus Armbruster
2014-12-04 9:34 ` Fam Zheng
2014-12-04 19:22 ` Eric Blake
@ 2014-12-10 8:31 ` Michael Tokarev
2 siblings, 0 replies; 8+ messages in thread
From: Michael Tokarev @ 2014-12-10 8:31 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: qemu-trivial
Applied to -trivial, thanks!
/mjt
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-12-10 8:31 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-04 9:26 [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup() Markus Armbruster
2014-12-04 9:34 ` Fam Zheng
2014-12-04 10:39 ` Markus Armbruster
2014-12-04 11:45 ` Fam Zheng
2014-12-04 12:43 ` Markus Armbruster
2014-12-04 19:17 ` Eric Blake
2014-12-04 19:22 ` Eric Blake
2014-12-10 8:31 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
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).