Re: [libvirt] [PATCH 4/8] tests: Use virAsprintf() to build titles
On Wed, 2019-03-13 at 10:32 +0100, Peter Krempa wrote:
> On Wed, Mar 13, 2019 at 10:25:16 +0100, Andrea Bolognani wrote:
> > On Wed, 2019-03-13 at 09:48 +0100, Peter Krempa wrote:
> > > On Thu, Mar 07, 2019 at 16:44:33 +0100, Andrea Bolognani wrote:
> > [...]
> > > > #define DO_TEST(arch, name) \
> > > > do { \
> > > > +VIR_AUTOFREE(char *) title = NULL; \
> > > > +VIR_AUTOFREE(char *) copyTitle = NULL; \
> > > > +if (virAsprintf(, "%s (%s)", name, arch) < 0 || \
> > > > +virAsprintf(, "copy %s (%s)", name, arch) < 0) {
> > > > \
> > > > +return -EXIT_FAILURE; \
> > >
> > > Coding style. Single-line body.
> >
> > There are multiple conditions with the same indentation, so per the
> > coding guidelines[1] the curly braces are required.
> >
> > Honesly, we should really give clang-format or whatever similar tool
> > a serious go and just start enforcing that code needs to be filtered
> > through it before being merged. Having humans worry about this kind
> > of nonsense is such an utter waste of time.
> >
> >
> > [1] https://libvirt.org/hacking.html#curly_braces, third example.
>
> Hmm, interresting. In this particular instance we are pretty much always
> breaking the style though. Majority of multi-line conditions with a
> single line body which I've encountered don't have the block.
Yeah, none of the style rules that doesn't have a corresponding
syntax-check rule is really enforced consistently, which is why we
should consider flipping the workflow and just have a tool reformat
the code for us in the first place.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/8] tests: Use virAsprintf() to build titles
On Wed, Mar 13, 2019 at 10:25:16 +0100, Andrea Bolognani wrote:
> On Wed, 2019-03-13 at 09:48 +0100, Peter Krempa wrote:
> > On Thu, Mar 07, 2019 at 16:44:33 +0100, Andrea Bolognani wrote:
> [...]
> > > #define DO_TEST(arch, name) \
> > > do { \
> > > +VIR_AUTOFREE(char *) title = NULL; \
> > > +VIR_AUTOFREE(char *) copyTitle = NULL; \
> > > +if (virAsprintf(, "%s (%s)", name, arch) < 0 || \
> > > +virAsprintf(, "copy %s (%s)", name, arch) < 0) { \
> > > +return -EXIT_FAILURE; \
> >
> > Coding style. Single-line body.
>
> There are multiple conditions with the same indentation, so per the
> coding guidelines[1] the curly braces are required.
>
> Honesly, we should really give clang-format or whatever similar tool
> a serious go and just start enforcing that code needs to be filtered
> through it before being merged. Having humans worry about this kind
> of nonsense is such an utter waste of time.
>
>
> [1] https://libvirt.org/hacking.html#curly_braces, third example.
Hmm, interresting. In this particular instance we are pretty much always
breaking the style though. Majority of multi-line conditions with a
single line body which I've encountered don't have the block.
ACK as is.
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/8] tests: Use virAsprintf() to build titles
On Wed, 2019-03-13 at 09:48 +0100, Peter Krempa wrote:
> On Thu, Mar 07, 2019 at 16:44:33 +0100, Andrea Bolognani wrote:
[...]
> > #define DO_TEST(arch, name) \
> > do { \
> > +VIR_AUTOFREE(char *) title = NULL; \
> > +VIR_AUTOFREE(char *) copyTitle = NULL; \
> > +if (virAsprintf(, "%s (%s)", name, arch) < 0 || \
> > +virAsprintf(, "copy %s (%s)", name, arch) < 0) { \
> > +return -EXIT_FAILURE; \
>
> Coding style. Single-line body.
There are multiple conditions with the same indentation, so per the
coding guidelines[1] the curly braces are required.
Honesly, we should really give clang-format or whatever similar tool
a serious go and just start enforcing that code needs to be filtered
through it before being merged. Having humans worry about this kind
of nonsense is such an utter waste of time.
[1] https://libvirt.org/hacking.html#curly_braces, third example.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/8] tests: Use virAsprintf() to build titles
On Thu, Mar 07, 2019 at 16:44:33 +0100, Andrea Bolognani wrote:
> We're using static string concatenation at the moment, but
> that will no longer be a possibility in a bit.
>
> Signed-off-by: Andrea Bolognani
> ---
> tests/qemucapabilitiestest.c | 11 ---
> tests/qemucaps2xmltest.c | 13 +
> 2 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c
> index e3c6681dd4..222ac05d79 100644
> --- a/tests/qemucapabilitiestest.c
> +++ b/tests/qemucapabilitiestest.c
> @@ -196,12 +196,17 @@ mymain(void)
>
> #define DO_TEST(arch, name) \
> do { \
> +VIR_AUTOFREE(char *) title = NULL; \
> +VIR_AUTOFREE(char *) copyTitle = NULL; \
> +if (virAsprintf(, "%s (%s)", name, arch) < 0 || \
> +virAsprintf(, "copy %s (%s)", name, arch) < 0) { \
> +return -EXIT_FAILURE; \
Coding style. Single-line body.
> +} \
> data.archName = arch; \
> data.base = name; \
> -if (virTestRun(name "(" arch ")", testQemuCaps, ) < 0) \
> +if (virTestRun(title, testQemuCaps, ) < 0) \
> data.ret = -1; \
> -if (virTestRun("copy " name "(" arch ")", \
> - testQemuCapsCopy, ) < 0) \
> +if (virTestRun(copyTitle, testQemuCapsCopy, ) < 0) \
> data.ret = -1; \
> } while (0)
ACK
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/8] tests: Use virAsprintf() to build titles
We're using static string concatenation at the moment, but
that will no longer be a possibility in a bit.
Signed-off-by: Andrea Bolognani
---
tests/qemucapabilitiestest.c | 11 ---
tests/qemucaps2xmltest.c | 13 +
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c
index e3c6681dd4..222ac05d79 100644
--- a/tests/qemucapabilitiestest.c
+++ b/tests/qemucapabilitiestest.c
@@ -196,12 +196,17 @@ mymain(void)
#define DO_TEST(arch, name) \
do { \
+VIR_AUTOFREE(char *) title = NULL; \
+VIR_AUTOFREE(char *) copyTitle = NULL; \
+if (virAsprintf(, "%s (%s)", name, arch) < 0 || \
+virAsprintf(, "copy %s (%s)", name, arch) < 0) { \
+return -EXIT_FAILURE; \
+} \
data.archName = arch; \
data.base = name; \
-if (virTestRun(name "(" arch ")", testQemuCaps, ) < 0) \
+if (virTestRun(title, testQemuCaps, ) < 0) \
data.ret = -1; \
-if (virTestRun("copy " name "(" arch ")", \
- testQemuCapsCopy, ) < 0) \
+if (virTestRun(copyTitle, testQemuCapsCopy, ) < 0) \
data.ret = -1; \
} while (0)
diff --git a/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c
index 46d2ce8b44..be460b42f8 100644
--- a/tests/qemucaps2xmltest.c
+++ b/tests/qemucaps2xmltest.c
@@ -197,10 +197,15 @@ mymain(void)
return EXIT_FAILURE;
#define DO_TEST(arch, name) \
-data.archName = arch; \
-data.base = name; \
-if (virTestRun(name "(" arch ")", testQemuCapsXML, ) < 0) \
-data.ret = -1
+do { \
+VIR_AUTOFREE(char *) title = NULL; \
+if (virAsprintf(, "%s (%s)", name, arch) < 0) \
+return -EXIT_FAILURE; \
+data.archName = arch; \
+data.base = name; \
+if (virTestRun(title, testQemuCapsXML, ) < 0) \
+data.ret = -1; \
+} while (0)
/* Keep this in sync with qemucapabilitiestest */
DO_TEST("x86_64", "caps_1.5.3");
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
