| Message ID | 20170424153244.2600-1-dgilbert@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 24 April 2017 at 16:32, Dr. David Alan Gilbert (git) <dgilbert@redhat.com> wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > The following changes since commit 4c55b1d0bad8a703f0499fe62e3761a0cd288da3: > > Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2017-04-24' into staging (2017-04-24 14:49:48 +0100) > > are available in the git repository at: > > git://github.com/dagrh/qemu.git tags/pull-hmp-20170424 > > for you to fetch changes up to e4e3992e626c4cc7514b271807c90f587771c646: > > tests: Add a tester for HMP commands (2017-04-24 15:55:35 +0100) > > ---------------------------------------------------------------- > HMP pull > > ---------------------------------------------------------------- clang doesn't like some code in test-hmp.c: /home/petmay01/linaro/qemu-for-merges/tests/test-hmp.c:138:9: error: logical not is only applied to the left hand side of this comparison [-Werror,-Wlogical-not-parentheses] if (!strcmp("isapc", mname) == 0 || !strcmp("puv3", mname) ^ ~~ /home/petmay01/linaro/qemu-for-merges/tests/test-hmp.c:138:9: note: add parentheses after the '!' to evaluate the comparison first if (!strcmp("isapc", mname) == 0 || !strcmp("puv3", mname) ^ ( ) /home/petmay01/linaro/qemu-for-merges/tests/test-hmp.c:138:9: note: add parentheses around left hand side expression to silence this warning if (!strcmp("isapc", mname) == 0 || !strcmp("puv3", mname) ^ ( ) /home/petmay01/linaro/qemu-for-merges/tests/test-hmp.c:140:12: error: logical not is only applied to the left hand side of this comparison [-Werror,-Wlogical-not-parentheses] || !strcmp("xenfv", mname) == 0 || !strcmp("xenpv", mname)) { ^ ~~ /home/petmay01/linaro/qemu-for-merges/tests/test-hmp.c:140:12: note: add parentheses after the '!' to evaluate the comparison first || !strcmp("xenfv", mname) == 0 || !strcmp("xenpv", mname)) { ^ ( ) /home/petmay01/linaro/qemu-for-merges/tests/test-hmp.c:140:12: note: add parentheses around left hand side expression to silence this warning || !strcmp("xenfv", mname) == 0 || !strcmp("xenpv", mname)) { ^ ( ) It does look rather odd: + /* Ignore blacklisted machines that have known problems */ + if (!strcmp("isapc", mname) == 0 || !strcmp("puv3", mname) + || !strcmp("tricore_testboard", mname) + || !strcmp("xenfv", mname) == 0 || !strcmp("xenpv", mname)) { + return; + } since it's not using the same kind of expression to test each board name -- is that deliberate, or accidental ? I think this expression means we'll actually skip every machine... thanks -- PMM
* Peter Maydell (peter.maydell@linaro.org) wrote: > On 24 April 2017 at 16:32, Dr. David Alan Gilbert (git) > <dgilbert@redhat.com> wrote: > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > The following changes since commit 4c55b1d0bad8a703f0499fe62e3761a0cd288da3: > > > > Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2017-04-24' into staging (2017-04-24 14:49:48 +0100) > > > > are available in the git repository at: > > > > git://github.com/dagrh/qemu.git tags/pull-hmp-20170424 > > > > for you to fetch changes up to e4e3992e626c4cc7514b271807c90f587771c646: > > > > tests: Add a tester for HMP commands (2017-04-24 15:55:35 +0100) > > > > ---------------------------------------------------------------- > > HMP pull > > > > ---------------------------------------------------------------- > > > clang doesn't like some code in test-hmp.c: > > /home/petmay01/linaro/qemu-for-merges/tests/test-hmp.c:138:9: error: > logical not is only applied to the left hand side of this comparison > [-Werror,-Wlogical-not-parentheses] > if (!strcmp("isapc", mname) == 0 || !strcmp("puv3", mname) > ^ ~~ <snip> > It does look rather odd: > > + /* Ignore blacklisted machines that have known problems */ > + if (!strcmp("isapc", mname) == 0 || !strcmp("puv3", mname) > + || !strcmp("tricore_testboard", mname) > + || !strcmp("xenfv", mname) == 0 || !strcmp("xenpv", mname)) { > + return; > + } > > since it's not using the same kind of expression to test > each board name -- is that deliberate, or accidental ? > > I think this expression means we'll actually skip every machine... Yep, you're right, just tried it with logging. That's accidental; hmm I should add a clang build somewhere. Thomas: Do you want to send me a fixed version? Dave > thanks > -- PMM -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 24.04.2017 18:57, Dr. David Alan Gilbert wrote: > * Peter Maydell (peter.maydell@linaro.org) wrote: >> On 24 April 2017 at 16:32, Dr. David Alan Gilbert (git) >> <dgilbert@redhat.com> wrote: >>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> >>> >>> The following changes since commit 4c55b1d0bad8a703f0499fe62e3761a0cd288da3: >>> >>> Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2017-04-24' into staging (2017-04-24 14:49:48 +0100) >>> >>> are available in the git repository at: >>> >>> git://github.com/dagrh/qemu.git tags/pull-hmp-20170424 >>> >>> for you to fetch changes up to e4e3992e626c4cc7514b271807c90f587771c646: >>> >>> tests: Add a tester for HMP commands (2017-04-24 15:55:35 +0100) >>> >>> ---------------------------------------------------------------- >>> HMP pull >>> >>> ---------------------------------------------------------------- >> >> >> clang doesn't like some code in test-hmp.c: >> >> /home/petmay01/linaro/qemu-for-merges/tests/test-hmp.c:138:9: error: >> logical not is only applied to the left hand side of this comparison >> [-Werror,-Wlogical-not-parentheses] >> if (!strcmp("isapc", mname) == 0 || !strcmp("puv3", mname) >> ^ ~~ > > <snip> > >> It does look rather odd: >> >> + /* Ignore blacklisted machines that have known problems */ >> + if (!strcmp("isapc", mname) == 0 || !strcmp("puv3", mname) >> + || !strcmp("tricore_testboard", mname) >> + || !strcmp("xenfv", mname) == 0 || !strcmp("xenpv", mname)) { >> + return; >> + } >> >> since it's not using the same kind of expression to test >> each board name -- is that deliberate, or accidental ? >> >> I think this expression means we'll actually skip every machine... > > Yep, you're right, just tried it with logging. Ouch, not sure how that happened ... looks like I used "strcmp("isapc", mname) == 0" in the first version of my patch, and then wanted to switch to "!strcmp()" when I added the xenfv and xenpv machines, but forgot to remove the "== 0" everywhere :-( Big sorry for that mess! > That's accidental; hmm I should add a clang build somewhere. > > Thomas: Do you want to send me a fixed version? Yes, I'll send a fixed version, where I also correct the memory leak that you noticed. Thomas
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> The following changes since commit 4c55b1d0bad8a703f0499fe62e3761a0cd288da3: Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2017-04-24' into staging (2017-04-24 14:49:48 +0100) are available in the git repository at: git://github.com/dagrh/qemu.git tags/pull-hmp-20170424 for you to fetch changes up to e4e3992e626c4cc7514b271807c90f587771c646: tests: Add a tester for HMP commands (2017-04-24 15:55:35 +0100) ---------------------------------------------------------------- HMP pull ---------------------------------------------------------------- Paolo Bonzini (1): hmp: gpa2hva and gpa2hpa hostaddr command Thomas Huth (3): libqtest: Ignore QMP events when parsing the response for HMP commands libqtest: Add a generic function to run a callback function for every machine tests: Add a tester for HMP commands hmp-commands.hx | 32 ++++++++++ monitor.c | 101 +++++++++++++++++++++++++++++++ tests/Makefile.include | 2 + tests/libqtest.c | 36 +++++++++++ tests/libqtest.h | 12 +++- tests/pc-cpu-test.c | 95 +++++++++++------------------ tests/qom-test.c | 36 ++--------- tests/test-hmp.c | 161 +++++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 385 insertions(+), 90 deletions(-) create mode 100644 tests/test-hmp.c