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;
> 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 git > > Could you try the following patch? Feel free to only apply the change > to readconfig.c if you want. > > -- > -Gene >I really don't understand; my apologies. There might be some potential improvement to the SYSAPPEND directive; fine. How is that an explanation to vesamenu.c32 failing, whereas menu.c32 and the CLI have no problems? And why would we assume that the problem in Fedora is related to the SYSAPPEND directive when Adam is having *success* with the current upstream git head? If there is some potential improvement in the code, that's great. I just fail to see the logic relating the original problematic behavior experienced by Adam in Fedora Rawhide (pre-F23) with the code for one directive working correctly in CLI and menu.c32 but failing with vesamenu.c32 (I am *not* talking about the code nor the patch themselves). TIA, Ady.> _______________________________________________ > Syslinux mailing list > Submissions to Syslinux at zytor.com > Unsubscribe or set options at: > http://www.zytor.com/mailman/listinfo/syslinux >
On 03.07.2015 12:28, Gene Cumm wrote:> 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 git > > Could you try the following patch? Feel free to only apply the change > to readconfig.c if you want. >It works OK as whole, for the current git: com32/include/menu.h | 2 +- com32/menu/readconfig.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 b7814be..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;
Hi, Ady wrote:> Maybe we should rather wait?This would normally my position, too. If it was only about some versions of SYSLINUX and gcc. But the symptoms are so strange that some larger problem with the switch from gcc 4.9 to gcc 5 has to be feared. This could affect any C program. My own ones too. I'm watching Red Hat bug 1234653 with astonishment.>From the viewpoint of SYSLINUX and general C programming,success of SYSLINUX production in a particular environment does not mean that all is well. If arbitrary (and supposedly correct) code changes can trigger the problem on gcc 5 or if gcc 4 suppressed a luring bug, then it will show up again sooner or later. It would be nice if one could point to some buggy version of gcc. But until this is proven, one cannot outrule that some unsafe C snippet in the source code misleads the compiler. poma wrote:> O Captain! my Captain! our fearful trip is done, > The ship has weather?d every rack, the prize we sought is wonThere is danger that the next ship can sink already when it leaves the harbour. Suspicion of teredo navalis. Adam Williamson wrote:> So in the end I actually wound up at the same commit poma did - but > exactly the other way around. [...] If I build 6.03 plus that > commit with GCC 5, it works.C compilation should really not behave this way. I do this since quite a while and hardly ever hit mad compilers. But ambiguous C code plus heavy optimizing can yield such results. Programmer's life in userspace is more comfortable. gdb and valgrind help a lot. I now ran on a git clone of SYSLINUX: cppcheck --force . > cppcheck.result 2>&1 It issues several accusations (search for "(error)" and "(warning)"). But i could not spot any relation to command line composition for the menu. Have a nice day :) Thomas
On Jul 3, 2015 1:34 PM, "poma" <pomidorabelisima at gmail.com> wrote:> It works OK as whole, for the current git:Excellent! This problem is solved. Now to commit a patch. --Gene
systemd Mailing List <systemd-devel at lists.freedesktop.org> would be the wrong address. On Mon, Jul 13, 2015 at 5:33 AM, poma <pomidorabelisima at gmail.com> wrote:> On 03.07.2015 21:35, Gene Cumm wrote: >> On Jul 3, 2015 1:34 PM, "poma" <pomidorabelisima at gmail.com> wrote: >> >>> It works OK as whole, for the current git: >> >> Excellent! This problem is solved. Now to commit a patch. >> >> --Gene >> > > Rather than syslinux-6.03.tar.xz with 75 patches, including this patch of yours - to produce a bootable system, > > git snapshot - http://repo.or.cz/w/syslinux.git/snapshot/5186539.tar.gz - i.e. syslinux-5186539.tar.gz > produces a bootable system even without this patch of yours, *but* not out-of-the-box. > > Now comes the interesting part, > isolinux-debug.bin must be engaged, vulgaris isolinux.bin won't do.That's quite odd that isolinux-debug.bin alone would solve it. Now I've learned Latin for common> Also using debug facility will break the ISOLINUX bootability: > sed -i '3 s/#//' mk/devel.mk > sed -i '4 s/# //' mk/devel.mk > sed -i '5 s/^/#/' mk/devel.mk > > > No matter what, the EXTLINUX is broken. > > > BIOS in question is the SeaBIOS > http://code.coreboot.org/p/seabiosGood to note though it's not strictly that BIOS from what I've seen. On Sun, Jul 12, 2015 at 8:58 AM, Gene Cumm <gene.cumm at gmail.com> wrote:> On Thu, Jul 9, 2015 at 4:57 PM, William Kennington via Syslinux > <syslinux at zytor.com> wrote: >> Still not working with gcc5.2 rc > > core/fs/diskio_bios.c:395 contains pm_fs_init() which I believe is the > first protected-mode code. > > void pm_fs_init(com32sys_t *regs) > { > static struct bios_disk_private priv; > > writechr('^'); > priv.regs = regs; > fs_init((const struct fs_ops **)regs->eax.l, (void *)&priv); > } > > > With the above, I see the caret. > > The call to this function in ISOLINUX is in core/isolinux.asm:1163 > > .common: > mov ecx,[Hidden] > mov ebx,[Hidden+4] > mov si,[bsHeads] > mov di,[bsSecPerTrack] > push ax > mov al,'$' > call writechr > pop ax > pm_call pm_fs_init > > > With the above, I see the dollar sign then the caret (it also appears > to need some indentation corrections). > > William, the above two simple changes may shed a glimmer of light on > what's going on.poma, please try the above code. If you need it in a proper patch, let me know. -- -Gene
On 13.07.2015 11:59, Gene Cumm wrote:> Wrong list. Replying to right list shortly. >For some reason I began to repeat the same mistake, mixing sys-linux and sys-temd, haha. At least one can laugh. Sorry for disturbance, it's summertime.
On 13.07.2015 12:08, Gene Cumm wrote:> systemd Mailing List <systemd-devel at lists.freedesktop.org> would be > the wrong address. > > On Mon, Jul 13, 2015 at 5:33 AM, poma <pomidorabelisima at gmail.com> wrote: >> On 03.07.2015 21:35, Gene Cumm wrote: >>> On Jul 3, 2015 1:34 PM, "poma" <pomidorabelisima at gmail.com> wrote: >>> >>>> It works OK as whole, for the current git: >>> >>> Excellent! This problem is solved. Now to commit a patch. >>> >>> --Gene >>> >> >> Rather than syslinux-6.03.tar.xz with 75 patches, including this patch of yours - to produce a bootable system, >> >> git snapshot - http://repo.or.cz/w/syslinux.git/snapshot/5186539.tar.gz - i.e. syslinux-5186539.tar.gz >> produces a bootable system even without this patch of yours, *but* not out-of-the-box. >> >> Now comes the interesting part, >> isolinux-debug.bin must be engaged, vulgaris isolinux.bin won't do. > > That's quite odd that isolinux-debug.bin alone would solve it. Now > I've learned Latin for common > >> Also using debug facility will break the ISOLINUX bootability: >> sed -i '3 s/#//' mk/devel.mk >> sed -i '4 s/# //' mk/devel.mk >> sed -i '5 s/^/#/' mk/devel.mk >> >> >> No matter what, the EXTLINUX is broken. >> >> >> BIOS in question is the SeaBIOS >> http://code.coreboot.org/p/seabios > > Good to note though it's not strictly that BIOS from what I've seen. > > > On Sun, Jul 12, 2015 at 8:58 AM, Gene Cumm <gene.cumm at gmail.com> wrote: >> On Thu, Jul 9, 2015 at 4:57 PM, William Kennington via Syslinux >> <syslinux at zytor.com> wrote: >>> Still not working with gcc5.2 rc >> >> core/fs/diskio_bios.c:395 contains pm_fs_init() which I believe is the >> first protected-mode code. >> >> void pm_fs_init(com32sys_t *regs) >> { >> static struct bios_disk_private priv; >> >> writechr('^'); >> priv.regs = regs; >> fs_init((const struct fs_ops **)regs->eax.l, (void *)&priv); >> } >> >> >> With the above, I see the caret. >> >> The call to this function in ISOLINUX is in core/isolinux.asm:1163 >> >> .common: >> mov ecx,[Hidden] >> mov ebx,[Hidden+4] >> mov si,[bsHeads] >> mov di,[bsSecPerTrack] >> push ax >> mov al,'$' >> call writechr >> pop ax >> pm_call pm_fs_init >> >> >> With the above, I see the dollar sign then the caret (it also appears >> to need some indentation corrections). >> >> William, the above two simple changes may shed a glimmer of light on >> what's going on. > > poma, please try the above code. If you need it in a proper patch, let me know. >core/fs/diskio_bios.c | 1 + core/isolinux.asm | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/core/fs/diskio_bios.c b/core/fs/diskio_bios.c index 7feb7fc..b7958c8 100644 --- a/core/fs/diskio_bios.c +++ b/core/fs/diskio_bios.c @@ -398,6 +398,7 @@ void pm_fs_init(com32sys_t *regs) { static struct bios_disk_private priv; + writechr('^'); priv.regs = regs; fs_init((const struct fs_ops **)regs->eax.l, (void *)&priv); } diff --git a/core/isolinux.asm b/core/isolinux.asm index 50d9fe1..9a5adf9 100644 --- a/core/isolinux.asm +++ b/core/isolinux.asm @@ -1160,6 +1160,10 @@ init_fs: mov ebx,[Hidden+4] mov si,[bsHeads] mov di,[bsSecPerTrack] + push ax + mov al,'$' + call writechr + pop ax pm_call pm_fs_init pm_call load_env32 enter_command: A patch like this?