[libvirt] [PATCH] virkeyfile: fix compilation error with clang
Clang complains about condition being always true: src/util/virkeyfile.c:113:23: error: result of comparison of constant 128 with expression of type 'const char' is always true [-Werror,-Wtautological-constant-out-of-range-compare] while (!IS_EOF && IS_ASCII(CUR) && CUR != ']') ^~~~~~~~~~~~~ src/util/virkeyfile.c:80:26: note: expanded from macro 'IS_ASCII' ~~~ ^ ~~~ Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virkeyfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c index 816bfae96d..ee29bd7aa6 100644 --- a/src/util/virkeyfile.c +++ b/src/util/virkeyfile.c @@ -77,7 +77,7 @@ struct _virKeyFileParserCtxt { #define IS_EOF (ctxt->cur >= ctxt->end) #define IS_EOL(c) (((c) == '\n') || ((c) == '\r')) #define IS_BLANK(c) (((c) == ' ') || ((c) == '\t')) -#define IS_ASCII(c) ((c) < 128) +#define IS_ASCII(c) (((unsigned char) c) < 128) #define CUR (*ctxt->cur) #define NEXT if (!IS_EOF) ctxt->cur++; -- 2.23.0
On 10 Dec 2019, at 15:11, Pavel Hrdina <phrdina@redhat.com> wrote:
Clang complains about condition being always true:
src/util/virkeyfile.c:113:23: error: result of comparison of constant 128 with expression of type 'const char' is always true [-Werror,-Wtautological-constant-out-of-range-compare] while (!IS_EOF && IS_ASCII(CUR) && CUR != ']') ^~~~~~~~~~~~~ src/util/virkeyfile.c:80:26: note: expanded from macro 'IS_ASCII' ~~~ ^ ~~~
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virkeyfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c index 816bfae96d..ee29bd7aa6 100644 --- a/src/util/virkeyfile.c +++ b/src/util/virkeyfile.c @@ -77,7 +77,7 @@ struct _virKeyFileParserCtxt { #define IS_EOF (ctxt->cur >= ctxt->end) #define IS_EOL(c) (((c) == '\n') || ((c) == '\r')) #define IS_BLANK(c) (((c) == ' ') || ((c) == '\t')) -#define IS_ASCII(c) ((c) < 128) +#define IS_ASCII(c) (((unsigned char) c) < 128)
Probably want parentheses around c.
#define CUR (*ctxt->cur) #define NEXT if (!IS_EOF) ctxt->cur++;
-- 2.23.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Dec 10, 2019 at 03:14:40PM +0100, Christophe de Dinechin wrote:
On 10 Dec 2019, at 15:11, Pavel Hrdina <phrdina@redhat.com> wrote:
Clang complains about condition being always true:
src/util/virkeyfile.c:113:23: error: result of comparison of constant 128 with expression of type 'const char' is always true [-Werror,-Wtautological-constant-out-of-range-compare] while (!IS_EOF && IS_ASCII(CUR) && CUR != ']') ^~~~~~~~~~~~~ src/util/virkeyfile.c:80:26: note: expanded from macro 'IS_ASCII' ~~~ ^ ~~~
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virkeyfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c index 816bfae96d..ee29bd7aa6 100644 --- a/src/util/virkeyfile.c +++ b/src/util/virkeyfile.c @@ -77,7 +77,7 @@ struct _virKeyFileParserCtxt { #define IS_EOF (ctxt->cur >= ctxt->end) #define IS_EOL(c) (((c) == '\n') || ((c) == '\r')) #define IS_BLANK(c) (((c) == ' ') || ((c) == '\t')) -#define IS_ASCII(c) ((c) < 128) +#define IS_ASCII(c) (((unsigned char) c) < 128)
Probably want parentheses around c.
Right, I'll fix it before pushing.
#define CUR (*ctxt->cur) #define NEXT if (!IS_EOF) ctxt->cur++;
-- 2.23.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Dec 10, 2019 at 03:29:43PM +0100, Pavel Hrdina wrote:
On Tue, Dec 10, 2019 at 03:14:40PM +0100, Christophe de Dinechin wrote:
On 10 Dec 2019, at 15:11, Pavel Hrdina <phrdina@redhat.com> wrote:
Clang complains about condition being always true:
src/util/virkeyfile.c:113:23: error: result of comparison of constant 128 with expression of type 'const char' is always true [-Werror,-Wtautological-constant-out-of-range-compare] while (!IS_EOF && IS_ASCII(CUR) && CUR != ']') ^~~~~~~~~~~~~ src/util/virkeyfile.c:80:26: note: expanded from macro 'IS_ASCII' ~~~ ^ ~~~
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virkeyfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c index 816bfae96d..ee29bd7aa6 100644 --- a/src/util/virkeyfile.c +++ b/src/util/virkeyfile.c @@ -77,7 +77,7 @@ struct _virKeyFileParserCtxt { #define IS_EOF (ctxt->cur >= ctxt->end) #define IS_EOL(c) (((c) == '\n') || ((c) == '\r')) #define IS_BLANK(c) (((c) == ' ') || ((c) == '\t')) -#define IS_ASCII(c) ((c) < 128) +#define IS_ASCII(c) (((unsigned char) c) < 128)
Probably want parentheses around c.
Right, I'll fix it before pushing.
with that change: Reviewed-by: Ján Tomko <jtomko@redhat.com> But the pre-existingcode seems odd - this is used to parse the INI-like syntax files for auth: https://libvirt.org/auth.html It does not really make sense to allow form feed or del in the group entry name. I guess the only remotely reasonable character we'd block by switching to g_ascii_isprint here is '\t' Jano
On Tue, Dec 10, 2019 at 03:57:59PM +0100, Ján Tomko wrote:
On Tue, Dec 10, 2019 at 03:29:43PM +0100, Pavel Hrdina wrote:
On Tue, Dec 10, 2019 at 03:14:40PM +0100, Christophe de Dinechin wrote:
On 10 Dec 2019, at 15:11, Pavel Hrdina <phrdina@redhat.com> wrote:
Clang complains about condition being always true:
src/util/virkeyfile.c:113:23: error: result of comparison of constant 128 with expression of type 'const char' is always true [-Werror,-Wtautological-constant-out-of-range-compare] while (!IS_EOF && IS_ASCII(CUR) && CUR != ']') ^~~~~~~~~~~~~ src/util/virkeyfile.c:80:26: note: expanded from macro 'IS_ASCII' ~~~ ^ ~~~
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virkeyfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c index 816bfae96d..ee29bd7aa6 100644 --- a/src/util/virkeyfile.c +++ b/src/util/virkeyfile.c @@ -77,7 +77,7 @@ struct _virKeyFileParserCtxt { #define IS_EOF (ctxt->cur >= ctxt->end) #define IS_EOL(c) (((c) == '\n') || ((c) == '\r')) #define IS_BLANK(c) (((c) == ' ') || ((c) == '\t')) -#define IS_ASCII(c) ((c) < 128) +#define IS_ASCII(c) (((unsigned char) c) < 128)
Probably want parentheses around c.
Right, I'll fix it before pushing.
with that change: Reviewed-by: Ján Tomko <jtomko@redhat.com>
Thanks, pushed now.
But the pre-existingcode seems odd - this is used to parse the INI-like syntax files for auth: https://libvirt.org/auth.html
It does not really make sense to allow form feed or del in the group entry name. I guess the only remotely reasonable character we'd block by switching to g_ascii_isprint here is '\t'
I was considering to "fix" the code as well but when looking into what GLib offers we can replace this whole file with GKeyFile [1]. Pavel [1] <https://developer.gnome.org/glib/stable/glib-Key-value-file-parser.html>
On Tue, Dec 10, 2019 at 04:03:56PM +0100, Pavel Hrdina wrote:
On Tue, Dec 10, 2019 at 03:57:59PM +0100, Ján Tomko wrote:
On Tue, Dec 10, 2019 at 03:29:43PM +0100, Pavel Hrdina wrote:
On Tue, Dec 10, 2019 at 03:14:40PM +0100, Christophe de Dinechin wrote:
On 10 Dec 2019, at 15:11, Pavel Hrdina <phrdina@redhat.com> wrote:
Clang complains about condition being always true:
src/util/virkeyfile.c:113:23: error: result of comparison of constant 128 with expression of type 'const char' is always true [-Werror,-Wtautological-constant-out-of-range-compare] while (!IS_EOF && IS_ASCII(CUR) && CUR != ']') ^~~~~~~~~~~~~ src/util/virkeyfile.c:80:26: note: expanded from macro 'IS_ASCII' ~~~ ^ ~~~
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virkeyfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c index 816bfae96d..ee29bd7aa6 100644 --- a/src/util/virkeyfile.c +++ b/src/util/virkeyfile.c @@ -77,7 +77,7 @@ struct _virKeyFileParserCtxt { #define IS_EOF (ctxt->cur >= ctxt->end) #define IS_EOL(c) (((c) == '\n') || ((c) == '\r')) #define IS_BLANK(c) (((c) == ' ') || ((c) == '\t')) -#define IS_ASCII(c) ((c) < 128) +#define IS_ASCII(c) (((unsigned char) c) < 128)
Probably want parentheses around c.
Right, I'll fix it before pushing.
with that change: Reviewed-by: Ján Tomko <jtomko@redhat.com>
Thanks, pushed now.
But the pre-existingcode seems odd - this is used to parse the INI-like syntax files for auth: https://libvirt.org/auth.html
It does not really make sense to allow form feed or del in the group entry name. I guess the only remotely reasonable character we'd block by switching to g_ascii_isprint here is '\t'
I was considering to "fix" the code as well but when looking into what GLib offers we can replace this whole file with GKeyFile [1].
Yes, if GKeyFile is compatible with what we're currently parsing, we should definitely just replace it Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 12/10/19 10:03 AM, Pavel Hrdina wrote:
On Tue, Dec 10, 2019 at 03:57:59PM +0100, Ján Tomko wrote:
On Tue, Dec 10, 2019 at 03:29:43PM +0100, Pavel Hrdina wrote:
On Tue, Dec 10, 2019 at 03:14:40PM +0100, Christophe de Dinechin wrote:
On 10 Dec 2019, at 15:11, Pavel Hrdina <phrdina@redhat.com> wrote:
Clang complains about condition being always true:
src/util/virkeyfile.c:113:23: error: result of comparison of constant 128 with expression of type 'const char' is always true [-Werror,-Wtautological-constant-out-of-range-compare] while (!IS_EOF && IS_ASCII(CUR) && CUR != ']') ^~~~~~~~~~~~~ src/util/virkeyfile.c:80:26: note: expanded from macro 'IS_ASCII' ~~~ ^ ~~~
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virkeyfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c index 816bfae96d..ee29bd7aa6 100644 --- a/src/util/virkeyfile.c +++ b/src/util/virkeyfile.c @@ -77,7 +77,7 @@ struct _virKeyFileParserCtxt { #define IS_EOF (ctxt->cur >= ctxt->end) #define IS_EOL(c) (((c) == '\n') || ((c) == '\r')) #define IS_BLANK(c) (((c) == ' ') || ((c) == '\t')) -#define IS_ASCII(c) ((c) < 128) +#define IS_ASCII(c) (((unsigned char) c) < 128)
Probably want parentheses around c.
Right, I'll fix it before pushing.
with that change: Reviewed-by: Ján Tomko <jtomko@redhat.com>
Thanks, pushed now.
But the pre-existingcode seems odd - this is used to parse the INI-like syntax files for auth: https://libvirt.org/auth.html
It does not really make sense to allow form feed or del in the group entry name. I guess the only remotely reasonable character we'd block by switching to g_ascii_isprint here is '\t'
I was considering to "fix" the code as well but when looking into what GLib offers we can replace this whole file with GKeyFile [1].
Pavel
[1] <https://developer.gnome.org/glib/stable/glib-Key-value-file-parser.html>
Ah cool. It would be nice to track a list somewhere of glib things that can replace internal libvirt infrastructure. Maybe a subsection of the bitesizedtasks wiki even though some of them will be not so bitesized. What are the other glib bits people know about? - Cole
participants (5)
-
Christophe de Dinechin -
Cole Robinson -
Daniel P. Berrangé -
Ján Tomko -
Pavel Hrdina