[libvirt] 2 patches: dead increment and 4 useless (always-false) disjuncts

[Jim Meyering]

Daniel Veillard wrote:
> On Mon, Sep 07, 2009 at 05:37:24PM +0200, Jim Meyering wrote:
>>
>> >From 74f57af2010f9cbe2315d625c6502a0b259e6802 Mon Sep 17 00:00:00 2001
>> From: Jim Meyering <meyering at redhat.com>
>> Date: Mon, 7 Sep 2009 10:03:22 +0200
>> Subject: [PATCH 1/2] xm_internal.c: remove dead increment of "data"
>>
>> * src/xm_internal.c (xenXMDomainConfigParse): Don't increment it.
>> ---
>>  src/xm_internal.c |    1 -
>>  1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/xm_internal.c b/src/xm_internal.c
>> index f206c8c..18613d4 100644
>> --- a/src/xm_internal.c
>> +++ b/src/xm_internal.c
>> @@ -1314,7 +1314,6 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) {
>>
>>                  if (!(data = strchr(key, '=')) || (data[0] == '\0'))
>>                      break;
>> -                data++;
>>
>>                  if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
>>                      if (STRPREFIX(key, "vncunused=")) {
>
>   trivial, ACK
>
>> >From d4db2dfaafbf827d1c8b8626a3dbdaa9f371e479 Mon Sep 17 00:00:00 2001
>> From: Jim Meyering <meyering at redhat.com>
>> Date: Mon, 7 Sep 2009 10:09:20 +0200
>> Subject: [PATCH 2/2] xm_internal.c: remove four useless comparisons after strchr
>>
>> * src/xm_internal.c (xenXMDomainConfigParse): After t=strchr...
>> don't test *t; it's known.  This was *not* detected by clang,
>> but I spotted it since once instance was in the vicinity of the
>> dead increment of "data".
>> ---
>>  src/xm_internal.c |    8 ++++----
>>  1 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/xm_internal.c b/src/xm_internal.c
>> index 18613d4..e7f6a55 100644
>> --- a/src/xm_internal.c
>> +++ b/src/xm_internal.c
>> @@ -862,7 +862,7 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) {
>>               */
>>
>>              /* Extract the source file path*/
>> -            if (!(offset = strchr(head, ',')) || offset[0] == '\0')
>> +            if (!(offset = strchr(head, ',')))
>>                  goto skipdisk;
>>              if ((offset - head) >= (PATH_MAX-1))
>>                  goto skipdisk;
>> @@ -882,7 +882,7 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) {
>>                  head = head + 6;
>>
>>              /* Extract the dest device name */
>> -            if (!(offset = strchr(head, ',')) || offset[0] == '\0')
>> +            if (!(offset = strchr(head, ',')))
>>                  goto skipdisk;
>>              if (VIR_ALLOC_N(disk->dst, (offset - head) + 1) < 0)
>>                  goto no_memory;
>> @@ -1018,7 +1018,7 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) {
>>                  char *data;
>>                  char *nextkey = strchr(key, ',');
>>
>> -                if (!(data = strchr(key, '=')) || (data[0] == '\0'))
>> +                if (!(data = strchr(key, '=')))
>>                      goto skipnic;
>>                  data++;
>>
>> @@ -1312,7 +1312,7 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) {
>>                      nextkey++;
>>                  }
>>
>> -                if (!(data = strchr(key, '=')) || (data[0] == '\0'))
>> +                if (!(data = strchr(key, '=')))
>>                      break;
>>
>>                  if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
>
>   Hum, I would like to understand the intent of the person who wrote
> this first, it's like one s trying to handle a case where strchr( ,c)
> could return the pointer to the end of string if c is not found or
> something like that. Weird ...

Dan?

git show 1b0f541704d0172fdbc4c0e075b37dc2e196d4cc

or visit this:
  http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=1b0f541704d0172fdb

e.g.,

  +            /* Extract the source */
  +            if (!(offset = index(head, ',')) || offset[0] == '\0')