Re: [PATCH] chainloader: Fix wrong break condition (must be AND not OR)

[C. Masloch]

On at 2018-02-06 16:41 +01:00, Daniel Kiper wrote:
>>> I am happy that you fix that issue but
>>>   https://en.wikipedia.org/wiki/Design_of_the_FAT_file_system#BPB331_OFS_15h
>>> shows that life is more complicated.
>>>
>>> Could you take that into account?
>>
>> MS-DOS 3.20 and 3.30 BPBs aren't supported anyway. The special case for
> 
> Aren't supported by whom? GRUB2?

Yes. If you look at include/grub/fat.h, the format is that which is
specified as "DOS 3.31" BPB by the Wikipedia article (plus FAT32 EBPB).
(Note that some of the offsets in the comments of struct grub_fat_bpb
are wrong. I'll change these in the patch v2.)

>> using a partition table entry's partition length field isn't applicable
>> here. (If it were, the function simply shouldn't check these fields.)
> 
> Please say why exactly it is not applicable here.

Because we are using these fields. If it was to be made applicable, it
should just not check the fields.

>> And this is the first I've read of that DR-DOS extension. (And again,
>> the simple solution to that one is also not to check the fields for zeros.)
>>
>>>> Tested with lDebug booted in qemu via grub2's
>>>> FreeDOS direct loading support, refer to
>>>> https://bitbucket.org/ecm/ldosboot + https://bitbucket.org/ecm/ldebug
>>>
>>> Could you put your SOB here?
>>
>> Like this?
>>
>> Signed-off-by: C. Masloch <address@hidden>
> 
> Yep.
> 
>> (Should I submit a PATCH v2 for this?)
> 
> Yes, please. I am happy to commit this patch if you provide
> such a nice and detailed explanation like above but with better
> formating. And please add some stuff which I asked for too.

Am about to mail the patch v2. (I found out how to make Thunderbird not
wrap long lines! mail.wrap_long_lines = false and mailnews.wraplength = 0.)

Regards,
ecm