Re: [PATCH] unused constants in i386/boot.h

[Pavel Roskin]

Quoting Yves Blusseau <address@hidden>:

> > As long as boot/i386/pc/boot.S has the variables whose offset these macros
> > are describing, I think it's fine to keep them.

I'd rather not keep the macros that are not used outside boot.S.  They  
give a wrong impression that somebody somewhere needs to know that  
offset.

As for the macros that are used, we should always have assembler  
statements ensuring that the described field is indeed there.

> boot/i386/pc/boot.S DON'T use this macros.
>
> They're only defined in i386/boot.h but they are not used anywhere. The
> new name of this constants are GRUB_BOOT_MACHINE_KERNEL_ADDR and
> GRUB_BOOT_MACHINE_KERNEL_SEG.
> So the GRUB_BOOT_MACHINE_KERNEL_ADDRESS and
> GRUB_BOOT_MACHINE_KERNEL_SEGMENT (see the difference with the constants
> below) constants can be definitively removed, and it's better to remove
> them to avoid confusion.

I believe we should use every such removal to simplify the code.  We  
can now use direct jump instead of an indirect.  That would hopefully  
save us two bytes we need to implement fat32 support.

I think we should also remove the version from boot.S, as it's not  
being checked anywhere.

I realize that somebody might suggest adding such check, but I'd  
rather append those to bytes at the end of the bootsector.  The extra  
bytes would be checked and stripped by grub-setup, so that they don't  
take space in the actual bootsector.  Also, if we add such check, we  
need a very clear message in i386/boot.h requiring the version to be  
updated whenever any change to the offsets is made.  The version  
number should be a plain integer, perhaps 16-bit, without distinction  
between major and minor versions, so that nobody needs to think where  
their change calls for a major version increase.

--
Regards,
Pavel Roskin