| Message ID | 87edfn8yn8.fsf@oldenburg.str.redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix C compatibility issue in mini-os | expand |
On 15.12.2023 12:59, Florian Weimer wrote: > The cc-option check always fails (that, it picks the second option > unconditionally) if the compiler does not support implicit conversion > from integers to pointers. Just drop the initialization because it > seems unnecessary in this context. Did you pay attention to ... > --- a/Config.mk > +++ b/Config.mk > @@ -21,7 +21,7 @@ endef > # of which would indicate an "unrecognized command-line option" warning/error. ... the comment the tail of which is visible here? Jan > # > # Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586) > -cc-option = $(shell if test -z "`echo 'void*p=1;' | \ > +cc-option = $(shell if test -z "`echo 'void*p;' | \ > $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- $(2) -`"; \ > then echo "$(2)"; else echo "$(3)"; fi ;) > > >
* Jan Beulich: > On 15.12.2023 12:59, Florian Weimer wrote: >> The cc-option check always fails (that, it picks the second option >> unconditionally) if the compiler does not support implicit conversion >> from integers to pointers. Just drop the initialization because it >> seems unnecessary in this context. > > Did you pay attention to ... > >> --- a/Config.mk >> +++ b/Config.mk >> @@ -21,7 +21,7 @@ endef >> # of which would indicate an "unrecognized command-line option" warning/error. > > ... the comment the tail of which is visible here? I didn't investigate how the mechanics of the selection are accomplished. I was so happy that I finally found the source of the int-conversion error and got a bit carried away. Looking at the shell script fragment: >> # Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586) >> -cc-option = $(shell if test -z "`echo 'void*p=1;' | \ >> +cc-option = $(shell if test -z "`echo 'void*p;' | \ >> $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- $(2) -`"; \ >> then echo "$(2)"; else echo "$(3)"; fi ;) I can see that the exit status of the compiler does not matter, so the patch will not make a difference. I'll record this as a false positive. Thanks, Florian
On 15.12.2023 13:23, Florian Weimer wrote: > * Jan Beulich: > >> On 15.12.2023 12:59, Florian Weimer wrote: >>> The cc-option check always fails (that, it picks the second option >>> unconditionally) if the compiler does not support implicit conversion >>> from integers to pointers. Just drop the initialization because it >>> seems unnecessary in this context. >> >> Did you pay attention to ... >> >>> --- a/Config.mk >>> +++ b/Config.mk >>> @@ -21,7 +21,7 @@ endef >>> # of which would indicate an "unrecognized command-line option" warning/error. >> >> ... the comment the tail of which is visible here? > > I didn't investigate how the mechanics of the selection are > accomplished. I was so happy that I finally found the source of the > int-conversion error and got a bit carried away. > > Looking at the shell script fragment: > >>> # Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586) >>> -cc-option = $(shell if test -z "`echo 'void*p=1;' | \ >>> +cc-option = $(shell if test -z "`echo 'void*p;' | \ >>> $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- $(2) -`"; \ >>> then echo "$(2)"; else echo "$(3)"; fi ;) > > I can see that the exit status of the compiler does not matter, so the > patch will not make a difference. An option may be to use Xen's (newer) variant cc-option = $(shell if $(1) $(2:-Wno-%=-W%) -Werror -c -o /dev/null -x c /dev/null >/dev/null 2>&1; \ then echo "$(2)"; else echo "$(3)"; fi ;) here as well (omitted all commentary). Cc-ing maintainers for possible input. Jan
On 15.12.23 13:29, Jan Beulich wrote: > On 15.12.2023 13:23, Florian Weimer wrote: >> * Jan Beulich: >> >>> On 15.12.2023 12:59, Florian Weimer wrote: >>>> The cc-option check always fails (that, it picks the second option >>>> unconditionally) if the compiler does not support implicit conversion >>>> from integers to pointers. Just drop the initialization because it >>>> seems unnecessary in this context. >>> >>> Did you pay attention to ... >>> >>>> --- a/Config.mk >>>> +++ b/Config.mk >>>> @@ -21,7 +21,7 @@ endef >>>> # of which would indicate an "unrecognized command-line option" warning/error. >>> >>> ... the comment the tail of which is visible here? >> >> I didn't investigate how the mechanics of the selection are >> accomplished. I was so happy that I finally found the source of the >> int-conversion error and got a bit carried away. >> >> Looking at the shell script fragment: >> >>>> # Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586) >>>> -cc-option = $(shell if test -z "`echo 'void*p=1;' | \ >>>> +cc-option = $(shell if test -z "`echo 'void*p;' | \ >>>> $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- $(2) -`"; \ >>>> then echo "$(2)"; else echo "$(3)"; fi ;) >> >> I can see that the exit status of the compiler does not matter, so the >> patch will not make a difference. > > An option may be to use Xen's (newer) variant > > cc-option = $(shell if $(1) $(2:-Wno-%=-W%) -Werror -c -o /dev/null -x c /dev/null >/dev/null 2>&1; \ > then echo "$(2)"; else echo "$(3)"; fi ;) > > here as well (omitted all commentary). Cc-ing maintainers for possible > input. There is no urgent need for that, as in Mini-OS I'm not aware of any use case to test for a "-W..." option, but the approach is really cleaner IMO. Juergen
diff --git a/Config.mk b/Config.mk index f2d1f0a..b7c9887 100644 --- a/Config.mk +++ b/Config.mk @@ -21,7 +21,7 @@ endef # of which would indicate an "unrecognized command-line option" warning/error. # # Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586) -cc-option = $(shell if test -z "`echo 'void*p=1;' | \ +cc-option = $(shell if test -z "`echo 'void*p;' | \ $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- $(2) -`"; \ then echo "$(2)"; else echo "$(3)"; fi ;)
The cc-option check always fails (that, it picks the second option unconditionally) if the compiler does not support implicit conversion from integers to pointers. Just drop the initialization because it seems unnecessary in this context. Related to: <https://fedoraproject.org/wiki/Changes/PortingToModernC> <https://fedoraproject.org/wiki/Toolchain/PortingToModernC> Signed-off-by: Florian Weimer <fweimer@redhat.com>