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

[Segher Boessenkool <segher@kernel.crashing.org>]

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.

> +   \ 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.

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

: 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.)


Segher