Bug#345931: A patch solving #345931

[Robert Millan]

On Tue, Sep 26, 2006 at 09:52:28AM -0400, Leandro Dorileo wrote:
> Hi Robert
> 
> On Monday 25 September 2006 10:17, Robert Millan wrote:
> > On Mon, Sep 11, 2006 at 06:26:37PM +0200, Mats Erik Andersson wrote:
> > >   Venerable Maintainers of Grub,
> > >
> > >  I offer you a patch that should close bug report
> > >  #345931.
> > >
> > >  The patch has been built on top of grub-0.97-13 with
> > >  success and it repaired with honours mbr-installs of
> > >
> > >         grub-0.97-1ubuntu1   on kubuntu 6.0.6
> > >   and
> > >         grub-0.95+cvs20040624-17sarge1 on Debian
> > > Sarge.
> >
> > Does your patch work with grub from debian sid?
> >
> > Does GRUB 2 also exhibit this problem?
> 
> I don`t think so, I`m trying this patch and I think It`s a totally grub legacy 
> related problem. I just have few considerations on the proposed patch. 

You mean that GRUB 2 works fine on the same hardware?

> > >    if (dest_drive & BIOS_FLAG_FIXED_DISK)
> > > -    /* Replace the jmp (2 bytes) with double nop's.  */
> > > -    *((unsigned short *) (stage1_buffer + STAGE1_BOOT_DRIVE_CHECK))
> > > -      = 0x9090;
> > > -
> > > +  {
> > > +    if ( *((unsigned char *) (stage1_buffer + STAGE1_BOOT_DRIVE_CHECK))
> > > == 0xeb ) +            /* For version 0.97:
> > > +	     * Replace the jmp (2 bytes) with double nop's.  */
> > > +       *((unsigned short *) (stage1_buffer + STAGE1_BOOT_DRIVE_CHECK))
> > > +         = 0x9090;
> 
> What have changed here?

Only indentation.  But we don't need to bother about this for Debian-specific
changes.  OTOH, see below..

> > > +    else if ( *((unsigned char *) (stage1_buffer +
> > > STAGE1_BOOT_DRIVE_CHECK)) == 0x80 ) +         {
> > > +            /* For versions 0.94-96:
> > > +	     *  Set the boot drive mask. This is a workaround for buggy BIOSes
> > > which +            *  don't pass boot drive correctly. Instead, they pass
> > > 0x00 even when +            *  booted from 0x80.
> > > +	     *  Rem: the old STAGE1_BOOT_DRIVE_MASK equals
> > > STAGE1_BOOT_DRIVE_CHECK + 2  */ +         *((unsigned char *)
> > > (stage1_buffer + STAGE1_BOOT_DRIVE_CHECK + 2 )) +             =
> > > (dest_drive & BIOS_FLAG_FIXED_DISK);
> > > +	  }
> > > +        else
> > > +	      /* The boot sector is older than version 0.94.
> > > +	       * Changing to a "nop" could make 0.92 and 0.93 acceptable. */
> > > +	   goto fail;
> 
> What about we set here the errnum with ERR_BAD_VERSION? or perhaps define a 
> new error with a new message - something like: you should run grub-install or 
> you will be unbootable - for this situation?

I think this kind of discussion really belongs in upstream.  That doesn't mean
we can't include the patch in debian if it fixes a problem, but all of this
should definitely be brought to bug-grub at gnu.org.

> > > +  }
> > >    /* Read the first sector of Stage 2.  */
> > >    disk_read_hook = disk_read_savesect_func;
> > >    if (grub_read (stage2_first_buffer, SECTOR_SIZE) != SECTOR_SIZE)
> 
> I`ll try this patch a bit more in the end of the week.

If you can confirm it works, then could you please resend it to bug-grub at gnu.org
(assuming you're the author) ?

-- 
Robert Millan

My spam trap is honeypot at aybabtu.com.  Note: this address is only intended for
spam harvesters.  Writing to it will get you added to my black list.