Syslinux - Jan 2019 - A bug in command localboot was introduced in version 6.03.

diff --git a/core/localboot.c b/core/localboot.c
index 0b8769e4..30bfb272 100644
--- a/core/localboot.c
+++ b/core/localboot.c
@@ -63,7 +63,6 @@ __export void local_boot(int16_t ax)
  	ireg.eax.w[0] = 0;	/* Reset drive */
  	__intcall(0x13, &ireg, NULL);

-	memset(&ireg, 0, sizeof(ireg));
  	ireg.eax.w[0] = 0x0201;	/* Read one sector */
  	ireg.ecx.w[0] = 0x0001;	/* C/H/S = 0/0/1 (first sector) */
  	ireg.ebx.w[0] = OFFS(trackbuf);

21.01.2019 11:03, Erwan Velu writes:
> Can you share the patch you used ?
>

On Mon, Jan 21, 2019 at 3:37 AM Victor Sologoubov via Syslinux
<syslinux at zytor.com> wrote:
>
> diff --git a/core/localboot.c b/core/localboot.c
> index 0b8769e4..30bfb272 100644
> --- a/core/localboot.c
> +++ b/core/localboot.c
> @@ -63,7 +63,6 @@ __export void local_boot(int16_t ax)
>         ireg.eax.w[0] = 0;      /* Reset drive */
>         __intcall(0x13, &ireg, NULL);
>
> -       memset(&ireg, 0, sizeof(ireg));
>         ireg.eax.w[0] = 0x0201; /* Read one sector */
>         ireg.ecx.w[0] = 0x0001; /* C/H/S = 0/0/1 (first sector) */
>         ireg.ebx.w[0] = OFFS(trackbuf);
>
> 21.01.2019 11:03, Erwan Velu writes:
> > Can you share the patch you used ?
I'd rather lean towards clearing state and having to set EDX/EAX again
but let's look at them all.

The first or second memset() seems redundant since it's never used in
between.

If we can assume when oreg is NULL intcall() never touches ireg, then
we don't need the third memset().  With the following code, it seems
it's a valid assumption:

    .no_copy:    mov edi,esi        ; Do a dummy copy-to-self


Erwan, thoughts?

-- 
-Gene

On Mon, Jan 21, 2019 at 9:20 AM Gene Cumm <gene.cumm at gmail.com>
wrote:
>
> On Mon, Jan 21, 2019 at 3:37 AM Victor Sologoubov via Syslinux
> <syslinux at zytor.com> wrote:
> >
> > diff --git a/core/localboot.c b/core/localboot.c
> > index 0b8769e4..30bfb272 100644
> > --- a/core/localboot.c
> > +++ b/core/localboot.c
> > @@ -63,7 +63,6 @@ __export void local_boot(int16_t ax)
> >         ireg.eax.w[0] = 0;      /* Reset drive */
> >         __intcall(0x13, &ireg, NULL);
> >
> > -       memset(&ireg, 0, sizeof(ireg));
> >         ireg.eax.w[0] = 0x0201; /* Read one sector */
> >         ireg.ecx.w[0] = 0x0001; /* C/H/S = 0/0/1 (first sector) */
> >         ireg.ebx.w[0] = OFFS(trackbuf);
> >
> > 21.01.2019 11:03, Erwan Velu writes:
> > > Can you share the patch you used ?
>
> I'd rather lean towards clearing state and having to set EDX/EAX again
> but let's look at them all.
>
> The first or second memset() seems redundant since it's never used in
between.
>
> If we can assume when oreg is NULL intcall() never touches ireg, then
> we don't need the third memset().  With the following code, it seems
> it's a valid assumption:
>
>     .no_copy:    mov edi,esi        ; Do a dummy copy-to-self
>
>
> Erwan, thoughts?
I'm proposing removing the first and third.  The first is redundant
and the third breaks things.  If the call on line 64

    __intcall(0x13, &ireg, NULL);

was instead

    __intcall(0x13, &ireg, &ireg);

we might have to worry about memset-ing then reloading EDX.

Along these lines, do we care the status results from this first call?
 Looking at commit b5274e00b8f4a74a5827e519e943d8c0594e7d82 which
seems to be the origin, it seems like we haven't cared in >17 years.

-- 
-Gene