| Message ID | 1287756396-13100-3-git-send-email-lcapitulino@redhat.com |
|---|---|
| State | New |
| Headers | show |
Luiz Capitulino <lcapitulino@redhat.com> writes: > From: Jan Kiszka <jan.kiszka@web.de> > > This avoids > > error: zero-length gnu_printf format string > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > --- > check-qjson.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/check-qjson.c b/check-qjson.c > index 0b60e45..64fcdcb 100644 > --- a/check-qjson.c > +++ b/check-qjson.c > @@ -639,7 +639,9 @@ END_TEST > > START_TEST(empty_input) > { > - QObject *obj = qobject_from_json(""); > + const char *empty = ""; > + > + QObject *obj = qobject_from_json(empty); > fail_unless(obj == NULL); > } > END_TEST The warning is silly. Printing nothing is unlikely to happen unintentionally, and is perfectly well-defined and portable. Why make the code ugly to avoid a useless warning, when we can disable the warning?
On Fri, 22 Oct 2010 19:15:07 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Luiz Capitulino <lcapitulino@redhat.com> writes: > > > From: Jan Kiszka <jan.kiszka@web.de> > > > > This avoids > > > > error: zero-length gnu_printf format string > > > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > > --- > > check-qjson.c | 4 +++- > > 1 files changed, 3 insertions(+), 1 deletions(-) > > > > diff --git a/check-qjson.c b/check-qjson.c > > index 0b60e45..64fcdcb 100644 > > --- a/check-qjson.c > > +++ b/check-qjson.c > > @@ -639,7 +639,9 @@ END_TEST > > > > START_TEST(empty_input) > > { > > - QObject *obj = qobject_from_json(""); > > + const char *empty = ""; > > + > > + QObject *obj = qobject_from_json(empty); > > fail_unless(obj == NULL); > > } > > END_TEST > > The warning is silly. Printing nothing is unlikely to happen > unintentionally, and is perfectly well-defined and portable. > > Why make the code ugly to avoid a useless warning, when we can disable > the warning? You mean, disable it only for this specific case or QEMU wide? If it's the former, please, submit a patch. Otherwise, this has been discussed already and the conclusion was that the warning is useful: http://www.mail-archive.com/qemu-devel@nongnu.org/msg44072.html Honestly speaking, no matter what the conclusion is, what can not happen is having code that doesn't compile in the tree. Either: we apply this patch or revert the patch that broke the build.
Am 22.10.2010 19:33, schrieb Luiz Capitulino: > On Fri, 22 Oct 2010 19:15:07 +0200 > Markus Armbruster<armbru@redhat.com> wrote: > > >> Luiz Capitulino<lcapitulino@redhat.com> writes: >> >> >>> From: Jan Kiszka<jan.kiszka@web.de> >>> >>> This avoids >>> >>> error: zero-length gnu_printf format string >>> >>> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com> >>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com> >>> --- >>> check-qjson.c | 4 +++- >>> 1 files changed, 3 insertions(+), 1 deletions(-) >>> >>> diff --git a/check-qjson.c b/check-qjson.c >>> index 0b60e45..64fcdcb 100644 >>> --- a/check-qjson.c >>> +++ b/check-qjson.c >>> @@ -639,7 +639,9 @@ END_TEST >>> >>> START_TEST(empty_input) >>> { >>> - QObject *obj = qobject_from_json(""); >>> + const char *empty = ""; >>> + >>> + QObject *obj = qobject_from_json(empty); >>> fail_unless(obj == NULL); >>> } >>> END_TEST >>> >> The warning is silly. Printing nothing is unlikely to happen >> unintentionally, and is perfectly well-defined and portable. >> >> Why make the code ugly to avoid a useless warning, when we can disable >> the warning? >> > You mean, disable it only for this specific case or QEMU wide? > > If it's the former, please, submit a patch. Otherwise, this has been > discussed already and the conclusion was that the warning is > useful: > > http://www.mail-archive.com/qemu-devel@nongnu.org/msg44072.html > > Honestly speaking, no matter what the conclusion is, what can not > happen is having code that doesn't compile in the tree. Either: we apply > this patch or revert the patch that broke the build. > If needed, commit 8b7968f7c4ac8c07cad6a1a0891d38cf239a2839 can be reverted partially (only for qjson.h). Tell me if you would prefer that solution, then I can send a patch.
On Fri, 22 Oct 2010 22:49:10 +0200 Stefan Weil <weil@mail.berlios.de> wrote: > Am 22.10.2010 19:33, schrieb Luiz Capitulino: > > On Fri, 22 Oct 2010 19:15:07 +0200 > > Markus Armbruster<armbru@redhat.com> wrote: > > > > > >> Luiz Capitulino<lcapitulino@redhat.com> writes: > >> > >> > >>> From: Jan Kiszka<jan.kiszka@web.de> > >>> > >>> This avoids > >>> > >>> error: zero-length gnu_printf format string > >>> > >>> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com> > >>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com> > >>> --- > >>> check-qjson.c | 4 +++- > >>> 1 files changed, 3 insertions(+), 1 deletions(-) > >>> > >>> diff --git a/check-qjson.c b/check-qjson.c > >>> index 0b60e45..64fcdcb 100644 > >>> --- a/check-qjson.c > >>> +++ b/check-qjson.c > >>> @@ -639,7 +639,9 @@ END_TEST > >>> > >>> START_TEST(empty_input) > >>> { > >>> - QObject *obj = qobject_from_json(""); > >>> + const char *empty = ""; > >>> + > >>> + QObject *obj = qobject_from_json(empty); > >>> fail_unless(obj == NULL); > >>> } > >>> END_TEST > >>> > >> The warning is silly. Printing nothing is unlikely to happen > >> unintentionally, and is perfectly well-defined and portable. > >> > >> Why make the code ugly to avoid a useless warning, when we can disable > >> the warning? > >> > > You mean, disable it only for this specific case or QEMU wide? > > > > If it's the former, please, submit a patch. Otherwise, this has been > > discussed already and the conclusion was that the warning is > > useful: > > > > http://www.mail-archive.com/qemu-devel@nongnu.org/msg44072.html > > > > Honestly speaking, no matter what the conclusion is, what can not > > happen is having code that doesn't compile in the tree. Either: we apply > > this patch or revert the patch that broke the build. > > > > > If needed, commit 8b7968f7c4ac8c07cad6a1a0891d38cf239a2839 > can be reverted partially (only for qjson.h). > > Tell me if you would prefer that solution, then I can send a patch. Well, the warning seems useful to me. It's the error checking test suite that's triggering it. Jan's fix looks file to me.
diff --git a/check-qjson.c b/check-qjson.c index 0b60e45..64fcdcb 100644 --- a/check-qjson.c +++ b/check-qjson.c @@ -639,7 +639,9 @@ END_TEST START_TEST(empty_input) { - QObject *obj = qobject_from_json(""); + const char *empty = ""; + + QObject *obj = qobject_from_json(empty); fail_unless(obj == NULL); } END_TEST