On Thu, 12/04 11:39, Markus Armbruster wrote:
> Fam Zheng <f...@redhat.com> writes:
>
> > On Thu, 12/04 10:26, Markus Armbruster wrote:
> >> Signed-off-by: Markus Armbruster <arm...@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 <f...@redhat.com>
if maintainers are happy with the indentation.
Fam
