On Tue, Sep 08, 2009 at 10:35:07AM +0200, Daniel Veillard wrote:
> On Mon, Sep 07, 2009 at 06:33:13PM +0200, Jim Meyering wrote:
> > 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@xxxxxxxxxx>
> > >> 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@xxxxxxxxxx>
> > >> 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')
>
> Man index is a bit confusing as it uses NULL to designate the '\0'
> character at the end of the string and the NULL pointer. Maybe this led
> to some confusion,
>
> Your patch sounds right, and Dan probably don't remember the obscure
> details of that patch 2.5 years ago :-)
With the horrible 'xm' driver my policy is always that the test cases
are correct and the driver code is broken unless proven otherwise.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list