Re: [PATCH SLOF 3/5] disk-label: introduce helper to check fat filesystem

[Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>]

Hi Segher,

Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Mon, Jun 22, 2015 at 01:29:45PM +0530, Nikunj A Dadhania wrote:
>> +: has-fat-filesystem ( block -- true | false )
>> +   \ block 0 byte 0-2 is a jump instruction in all FAT
>> +   \ filesystems.
>
> "block" there is not a block number, just a host address.  So it's not
> a good name.  Maybe do a better name for this word as well, something
> saying it looks at a disk block.

Sure.

>
>> +   \ e9 and eb are jump instructions in x86 assembler.
>> +   dup c@ e9 <> IF
>> +      dup c@ eb <> swap
>> +      2+  c@ 90 <> or
>> +      IF false EXIT THEN
>> +   ELSE DROP THEN
>> +   TRUE
>> +;
>
> Don't write DROP and TRUE in caps please.  The purpose of having the
> structure words in caps is to make them stand out more, to make things
> more readable; putting other things in caps as well destroys that.

Sure, will take care.

> Since you factored this, it becomes more readable if you invert the
> conditions:

Sure.

> : fat-bootblock? ( addr -- flag )
>    \ byte 0-2 of the bootblock is a jump instruction in
>    \ all FAT filesystems.
>    \ e9 and eb are jump instructions in x86 assembler.
>    dup c@ e9 = IF drop true EXIT THEN
>    dup c@ eb = swap 2+ c@ 90 = and ;
>
> (not tested, etc.)
Will test.

Regards,
Nikunj