[v1] Misc changes to allow building with Windows Clang

[PATCH 3/7] osdep: Remove unused strings.h

Posted by Kostiantyn Kostiuk 1 week ago

Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
---
 include/qemu/osdep.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index f151578b5c..2f0e61ad6b 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -114,7 +114,6 @@ QEMU_EXTERN_C int daemon(int, int);
 #include <stdio.h>
 
 #include <string.h>
-#include <strings.h>
 #include <inttypes.h>
 #include <limits.h>
 /* Put unistd.h before time.h as that triggers localtime_r/gmtime_r
-- 
2.52.0

Re: [PATCH 3/7] osdep: Remove unused strings.h

Posted by Peter Maydell 1 week ago

On Thu, 26 Mar 2026 at 16:02, Kostiantyn Kostiuk <kkostiuk@redhat.com> wrote:
>
> Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
> ---
>  include/qemu/osdep.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index f151578b5c..2f0e61ad6b 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -114,7 +114,6 @@ QEMU_EXTERN_C int daemon(int, int);
>  #include <stdio.h>
>
>  #include <string.h>
> -#include <strings.h>
>  #include <inttypes.h>
>  #include <limits.h>
>  /* Put unistd.h before time.h as that triggers localtime_r/gmtime_r

This needs more rationale about why we can drop the header.
POSIX says it brings in ffs(), strncasecmp_l() and strcasecmp_l(),
which we don't use, but also strcasecmp() and strncasecmp()
which we definitely do use.

Some libcs (notably glibc) may also define those in string.h,
but the manpage says that's a backward-compatibility thing,
so we shouldn't rely on it.

-- PMM

Re: [PATCH 3/7] osdep: Remove unused strings.h

Posted by Kostiantyn Kostiuk 1 week ago

On Thu, Mar 26, 2026 at 6:20 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Thu, 26 Mar 2026 at 16:02, Kostiantyn Kostiuk <kkostiuk@redhat.com>
> wrote:
> >
> > Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
> > ---
> >  include/qemu/osdep.h | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> > index f151578b5c..2f0e61ad6b 100644
> > --- a/include/qemu/osdep.h
> > +++ b/include/qemu/osdep.h
> > @@ -114,7 +114,6 @@ QEMU_EXTERN_C int daemon(int, int);
> >  #include <stdio.h>
> >
> >  #include <string.h>
> > -#include <strings.h>
> >  #include <inttypes.h>
> >  #include <limits.h>
> >  /* Put unistd.h before time.h as that triggers localtime_r/gmtime_r
>
> This needs more rationale about why we can drop the header.
> POSIX says it brings in ffs(), strncasecmp_l() and strcasecmp_l(),
> which we don't use, but also strcasecmp() and strncasecmp()
> which we definitely do use.
>

Will be ok if I replace strcasecmp with g_ascii_strcasecmp (and other
functions)
in the full QEMU tree?

Currently, I see in QEMU we have a mix of strcasecmp/g_ascii_strcasecmp.
In some places we use glb2, in some libc.


> Some libcs (notably glibc) may also define those in string.h,
> but the manpage says that's a backward-compatibility thing,
> so we shouldn't rely on it.
>
> -- PMM
>
>

Re: [PATCH 3/7] osdep: Remove unused strings.h

Posted by Peter Maydell 1 week ago

On Thu, 26 Mar 2026 at 16:32, Kostiantyn Kostiuk <kkostiuk@redhat.com> wrote:
>
>
>
>
>
> On Thu, Mar 26, 2026 at 6:20 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On Thu, 26 Mar 2026 at 16:02, Kostiantyn Kostiuk <kkostiuk@redhat.com> wrote:
>> >
>> > Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
>> > ---
>> >  include/qemu/osdep.h | 1 -
>> >  1 file changed, 1 deletion(-)
>> >
>> > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>> > index f151578b5c..2f0e61ad6b 100644
>> > --- a/include/qemu/osdep.h
>> > +++ b/include/qemu/osdep.h
>> > @@ -114,7 +114,6 @@ QEMU_EXTERN_C int daemon(int, int);
>> >  #include <stdio.h>
>> >
>> >  #include <string.h>
>> > -#include <strings.h>
>> >  #include <inttypes.h>
>> >  #include <limits.h>
>> >  /* Put unistd.h before time.h as that triggers localtime_r/gmtime_r
>>
>> This needs more rationale about why we can drop the header.
>> POSIX says it brings in ffs(), strncasecmp_l() and strcasecmp_l(),
>> which we don't use, but also strcasecmp() and strncasecmp()
>> which we definitely do use.
>
>
> Will be ok if I replace strcasecmp with g_ascii_strcasecmp (and other functions)
> in the full QEMU tree?
>
> Currently, I see in QEMU we have a mix of strcasecmp/g_ascii_strcasecmp.
> In some places we use glb2, in some libc.

Yes, it would be OK, but we need to be a bit careful, because
strcasecmp() and g_ascii_strcasecmp() aren't identical; the
former honours locale and the latter is a pure ASCII comparison.
So we need to check that that's what we want. A quick look at
the grep results suggests that in fact we really do want an
ASCII comparison in all these places (and the locale stuff
in strcasecmp can be a source of bugs, as noted e.g. here:
https://daniel.haxx.se/blog/2008/10/15/strcasecmp-in-turkish/ );
but it does need a little bit of a check of the context where
the functions are used. Where we're doing something like "check
this input value matches 'off'" this is a code cleanup regardless
of portability requirements.

thanks
-- PMM

Re: [PATCH 3/7] osdep: Remove unused strings.h

Posted by Kostiantyn Kostiuk 1 week ago

On Thu, Mar 26, 2026 at 6:41 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Thu, 26 Mar 2026 at 16:32, Kostiantyn Kostiuk <kkostiuk@redhat.com>
> wrote:
> >
> >
> >
> >
> >
> > On Thu, Mar 26, 2026 at 6:20 PM Peter Maydell <peter.maydell@linaro.org>
> wrote:
> >>
> >> On Thu, 26 Mar 2026 at 16:02, Kostiantyn Kostiuk <kkostiuk@redhat.com>
> wrote:
> >> >
> >> > Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
> >> > ---
> >> >  include/qemu/osdep.h | 1 -
> >> >  1 file changed, 1 deletion(-)
> >> >
> >> > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> >> > index f151578b5c..2f0e61ad6b 100644
> >> > --- a/include/qemu/osdep.h
> >> > +++ b/include/qemu/osdep.h
> >> > @@ -114,7 +114,6 @@ QEMU_EXTERN_C int daemon(int, int);
> >> >  #include <stdio.h>
> >> >
> >> >  #include <string.h>
> >> > -#include <strings.h>
> >> >  #include <inttypes.h>
> >> >  #include <limits.h>
> >> >  /* Put unistd.h before time.h as that triggers localtime_r/gmtime_r
> >>
> >> This needs more rationale about why we can drop the header.
> >> POSIX says it brings in ffs(), strncasecmp_l() and strcasecmp_l(),
> >> which we don't use, but also strcasecmp() and strncasecmp()
> >> which we definitely do use.
> >
> >
> > Will be ok if I replace strcasecmp with g_ascii_strcasecmp (and other
> functions)
> > in the full QEMU tree?
> >
> > Currently, I see in QEMU we have a mix of strcasecmp/g_ascii_strcasecmp.
> > In some places we use glb2, in some libc.
>
> Yes, it would be OK, but we need to be a bit careful, because
> strcasecmp() and g_ascii_strcasecmp() aren't identical; the
> former honours locale and the latter is a pure ASCII comparison.
> So we need to check that that's what we want. A quick look at
> the grep results suggests that in fact we really do want an
> ASCII comparison in all these places (and the locale stuff
> in strcasecmp can be a source of bugs, as noted e.g. here:
> https://daniel.haxx.se/blog/2008/10/15/strcasecmp-in-turkish/ );
> but it does need a little bit of a check of the context where
> the functions are used. Where we're doing something like "check
> this input value matches 'off'" this is a code cleanup regardless
> of portability requirements.
>
>
Is this ok to update several strcasecmp usage in one commit,
or better to split it per component?


> thanks
> -- PMM
>
>

Re: [PATCH 3/7] osdep: Remove unused strings.h

Posted by Peter Maydell 1 week ago

On Thu, 26 Mar 2026 at 18:26, Kostiantyn Kostiuk <kkostiuk@redhat.com> wrote:
>
>
>
> On Thu, Mar 26, 2026 at 6:41 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On Thu, 26 Mar 2026 at 16:32, Kostiantyn Kostiuk <kkostiuk@redhat.com> wrote:
>> > Will be ok if I replace strcasecmp with g_ascii_strcasecmp (and other functions)
>> > in the full QEMU tree?
>> >
>> > Currently, I see in QEMU we have a mix of strcasecmp/g_ascii_strcasecmp.
>> > In some places we use glb2, in some libc.
>>
>> Yes, it would be OK, but we need to be a bit careful, because
>> strcasecmp() and g_ascii_strcasecmp() aren't identical; the
>> former honours locale and the latter is a pure ASCII comparison.
>> So we need to check that that's what we want. A quick look at
>> the grep results suggests that in fact we really do want an
>> ASCII comparison in all these places (and the locale stuff
>> in strcasecmp can be a source of bugs, as noted e.g. here:
>> https://daniel.haxx.se/blog/2008/10/15/strcasecmp-in-turkish/ );
>> but it does need a little bit of a check of the context where
>> the functions are used. Where we're doing something like "check
>> this input value matches 'off'" this is a code cleanup regardless
>> of portability requirements.
>>
>
> Is this ok to update several strcasecmp usage in one commit,
> or better to split it per component?

I think I would split it per component, because the people
who might care about it are different in each case.

-- PMM