Hi, hpa wrote:> On PowerPC (I think) "unsigned char" is the default.In any case it seems a good idea to interpret the character more explicitely. To my experience, one signdness change causes a little tree of consequential signedness changes or questionable cast operations. How about the following instead ? if ((c >= 0 && c <= ' ') || c == '\x7f') { Besides code improvement there is the riddle of how gcc 5 can spoil the result.> gcc does have an -funsigned-char option.So question to poma: Can it be that the tested installation of gcc 5 uses this option by default ? I read through the mailing list archive but could not find a mail where you tell the compiler version which produces working binaries. Additionally to the different signedness of char, this theory would also need (unsigned) characters between 128 and 255 in the SYSAPPEND variables. Is this plausible ? Have a nice day :) Thomas
On 07/02/2015 02:12 PM, Thomas Schmitt via Syslinux wrote:> Hi, > > hpa wrote: >> On PowerPC (I think) "unsigned char" is the default. > > In any case it seems a good idea to interpret the character > more explicitely. To my experience, one signdness change causes > a little tree of consequential signedness changes or questionable > cast operations. > How about the following instead ? > > if ((c >= 0 && c <= ' ') || c == '\x7f') { >First of all, this should be an inline instead of being open-coded. I thought we had a myisspace() routine already... -hpa
On Thu, Jul 2, 2015 at 6:14 PM, H. Peter Anvin via Syslinux <syslinux at zytor.com> wrote:> On 07/02/2015 02:12 PM, Thomas Schmitt via Syslinux wrote: >> Hi, >> >> hpa wrote: >>> On PowerPC (I think) "unsigned char" is the default. >> >> In any case it seems a good idea to interpret the character >> more explicitely. To my experience, one signdness change causes >> a little tree of consequential signedness changes or questionable >> cast operations. >> How about the following instead ? >> >> if ((c >= 0 && c <= ' ') || c == '\x7f') { >> > > First of all, this should be an inline instead of being open-coded. I > thought we had a myisspace() routine already...Quick git grep shows isspace() (com32/include/ctype.h:90 and core/include/ctype.h:25) and my_isspace() (com32/include/menu.h:196) -- -Gene
On 02.07.2015 23:12, Thomas Schmitt wrote:> Hi, > > hpa wrote: >> On PowerPC (I think) "unsigned char" is the default. > > In any case it seems a good idea to interpret the character > more explicitely. To my experience, one signdness change causes > a little tree of consequential signedness changes or questionable > cast operations. > How about the following instead ? > > if ((c >= 0 && c <= ' ') || c == '\x7f') { >- "unsigned char c;" does not solve the problem - "c >= 0 && c <= ' '" solves the problem for the current git --- a/com32/menu/readconfig.c +++ b/com32/menu/readconfig.c @@ -299,7 +299,7 @@ char c; while ((c = *src++)) { - if (c <= ' ' || c == '\x7f') { + if ((c >= 0 && c <= ' ') || c == '\x7f') {> > Besides code improvement there is the riddle of how gcc 5 > can spoil the result. > >> gcc does have an -funsigned-char option. > > So question to poma: > > Can it be that the tested installation of gcc 5 uses this > option by default ?See for yourself is it so or not: $ rpm -qf /usr/lib/rpm/rpmrc rpm-4.12.0.1-17.fc23.x86_64 $ grep optflags /usr/lib/rpm/rpmrc | grep 86 optflags: fat -O2 -g -arch i386 -arch ppc optflags: i386 -O2 -g -march=i386 -mtune=i686 optflags: i486 -O2 -g -march=i486 optflags: i586 -O2 -g -march=i586 optflags: i686 -O2 -g -march=i686 optflags: x86_64 -O2 -g $ rpm -qf /usr/lib/rpm/redhat/rpmrc redhat-rpm-config-33-2.fc23.noarch $ grep optflags /usr/lib/rpm/redhat/rpmrc | grep 86 optflags: i386 %{__global_cflags} -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables optflags: i486 %{__global_cflags} -m32 -march=i486 -fasynchronous-unwind-tables optflags: i586 %{__global_cflags} -m32 -march=i586 -mtune=generic -fasynchronous-unwind-tables optflags: i686 %{__global_cflags} -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables optflags: x86_64 %{__global_cflags} -m64 -mtune=generic $ grep ' RPM_OPT_FLAGS\| RPM_PACKAGE_NAME' /var/tmp/rpm-tmp.XNVZuk RPM_OPT_FLAGS="-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic" RPM_PACKAGE_NAME="syslinux"> I read through the mailing list archive but could not > find a mail where you tell the compiler version which > produces working binaries.I wrote it in this thread already, here I will repeat a reference for you: https://bugzilla.redhat.com/show_bug.cgi?id=1234653#c9> > Additionally to the different signedness of char, this theory > would also need (unsigned) characters between 128 and 255 in > the SYSAPPEND variables. Is this plausible ? > > > Have a nice day :) > > Thomas >
Hi, poma wrote:> - "unsigned char c;" does not solve the problem > > - "c >= 0 && c <= ' '" solves the problem for the current gitWhat the ... ?!? My proposal of "(c >= 0 ... )" was only made to avoid the need for a type change of variable "c". Both proposals should be equivalent ... on the first morning glimpse they still are.> > could not > > find a mail where you tell the compiler version which > > produces working binaries.> I wrote it in this thread already, here I will repeat a reference for you: > https://bugzilla.redhat.com/show_bug.cgi?id=1234653#c94.9.2-5.fc22. I got about the same one. Debian 4.9.2-10. Anybody can give me hints how to install gcc 5 as alternative compiler on a Debian 8.1 ? (I am still learning about my new system.) To poma: There must be a character between 128 and 255 (-128 to -1) in the SYSAPPEND strings which needs the "c >= 0" term to survive. Any idea what it could be in the light of the list in com32/include/syslinux/sysappend.h ? To all: Any idea how to inspect those strings while booting or while an operating system is up ? Have a nice day :) Thomas
On Fri, Jul 03, 2015 at 06:50:27AM +0200, poma via Syslinux wrote:
<snip/> > $ rpm -qf /usr/lib/rpm/redhat/rpmrc
> redhat-rpm-config-33-2.fc23.noarch
>
> $ grep optflags /usr/lib/rpm/redhat/rpmrc | grep 86
> optflags: i386 %{__global_cflags} -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables
> optflags: i486 %{__global_cflags} -m32 -march=i486
-fasynchronous-unwind-tables
> optflags: i586 %{__global_cflags} -m32 -march=i586 -mtune=generic
-fasynchronous-unwind-tables
> optflags: i686 %{__global_cflags} -m32 -march=i686 -mtune=atom
-fasynchronous-unwind-tables
> optflags: x86_64 %{__global_cflags} -m64 -mtune=generic
>
>
> $ grep ' RPM_OPT_FLAGS\| RPM_PACKAGE_NAME'
/var/tmp/rpm-tmp.XNVZuk
> RPM_OPT_FLAGS="-O2 -g -pipe -Wall -Werror=format-security
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong
--param=ssp-buffer-size=4 -grecord-gcc-switches
-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic"
> RPM_PACKAGE_NAME="syslinux"
I, sysadmin, not a hardcore programmer, have a odd feeling with the -m64 flag.
For Syslinux, the bootloader, I do expect -m32. I know 16-bits, I80286, is over.
Thank you for reading my worries.
Groeten
Geert Stappers
--
Leven en laten leven
------------- volgend deel ------------
Een niet-tekst bijlage is gescrubt...
Naam: signature.asc
Type: application/pgp-signature
Grootte: 836 bytes
Omschrijving: Digital signature
URL :
<http://www.zytor.com/pipermail/syslinux/attachments/20150703/f1a17081/attachment.sig>
On Fri, Jul 3, 2015 at 12:50 AM, poma via Syslinux <syslinux at zytor.com> wrote:> - "unsigned char c;" does not solve the problem > > - "c >= 0 && c <= ' '" solves the problem for the current gitCould you try the following patch? Feel free to only apply the change to readconfig.c if you want. -- -Gene diff --git a/com32/include/menu.h b/com32/include/menu.h index bc0182f..b0251e4 100644 --- a/com32/include/menu.h +++ b/com32/include/menu.h @@ -195,7 +195,7 @@ void local_cursor_enable(bool); static inline int my_isspace(char c) { - return (unsigned char)c <= ' '; + return (unsigned char)c <= ' ' || (unsigned char)c == '\x7f'; } int my_isxdigit(char c); diff --git a/com32/menu/readconfig.c b/com32/menu/readconfig.c index 257b042..a433fad 100644 --- a/com32/menu/readconfig.c +++ b/com32/menu/readconfig.c @@ -299,7 +299,7 @@ static char *copy_sysappend_string(char *dst, const char *src) char c; while ((c = *src++)) { - if (c <= ' ' || c == '\x7f') { + if (my_isspace(c)) { if (!was_space) *dst++ = '_'; was_space = true;