On 10 Dec 2019, at 15:11, Pavel Hrdina <phrdina(a)redhat.com&gt; 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(a)redhat.com&gt; --- 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 -- libvir-list mailing list libvir-list(a)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(a)redhat.com&gt; 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(a)redhat.com&gt; > > --- > > 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.

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(a)redhat.com&gt; 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(a)redhat.com&gt; > > > > --- > > > > 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(a)redhat.com&gt; 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].