[SLOF] [PATCH v2 1/2] usb/storage: Invert the logic of the IF-statements

[Thomas Huth]

On 2018-12-12 15:31, Laurent Vivier wrote:
> to prepare write implementation
> 
> Signed-off-by: Laurent Vivier <lvivier at redhat.com>
> ---
>  slof/fs/usb/dev-storage.fs | 39 +++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 20 deletions(-)
> 
> diff --git a/slof/fs/usb/dev-storage.fs b/slof/fs/usb/dev-storage.fs
> index 94f8421..fe5af1c 100644
> --- a/slof/fs/usb/dev-storage.fs
> +++ b/slof/fs/usb/dev-storage.fs
> @@ -107,23 +107,23 @@ scsi-open
>      TO resp-size
>      TO resp-buffer
>      udev USB_PIPE_OUT td-buf td-buf-phys dma-buf-phys usb>cmd 1F
> -    usb-transfer-bulk IF \ transfer CBW
> -	resp-size IF
> -	    d# 125 us
> -	    udev USB_PIPE_IN td-buf td-buf-phys resp-buffer resp-size
> -	    usb-transfer-bulk 1 = not IF \ transfer data
> -	        usb-disk-debug?	IF ." Data phase failed " cr THEN
> -		\ FALSE EXIT
> -		\ in case of a stall/halted endpoint we clear the halt
> -		\ Fall through and try reading the CSW
> -	    THEN
> -	THEN
> -	d# 125 us
> -	udev USB_PIPE_IN td-buf td-buf-phys dma-buf-phys usb>csw 0D
> -	usb-transfer-bulk \ transfer CSW
> -    ELSE
> -	FALSE EXIT
> +    usb-transfer-bulk 1 = not IF

In case you respin: Replace "= not" with "<>".
(sorry for not noticing this in v1 already!)

> +        FALSE EXIT
>      THEN
> +    \ transfer CBW
> +    resp-size IF
> +        d# 125 us
> +        udev USB_PIPE_IN td-buf td-buf-phys resp-buffer resp-size
> +        usb-transfer-bulk 1 = not IF \ transfer data

You only moved the code, but in case you respin, "= not" could be
replaced with "<>" here, too.

> +            usb-disk-debug? IF ." Data phase failed " cr THEN
> +            \ FALSE EXIT
> +            \ in case of a stall/halted endpoint we clear the halt
> +            \ Fall through and try reading the CSW
> +        THEN
> +    THEN
> +    d# 125 us
> +    udev USB_PIPE_IN td-buf td-buf-phys dma-buf-phys usb>csw 0D
> +    usb-transfer-bulk \ transfer CSW
>  ;
>  
>  STRUCT \ cbw
> @@ -189,12 +189,11 @@ CONSTANT cbw-length
>  
>      \ Send it
>      dma-buf-phys usb>data usb-buf-len
> -    do-bulk-command IF
> -	dma-buf usb>data usb-buf-addr usb-buf-len move
> -    ELSE
> -        ." USB-DISK: Bulk commad failed!" cr
> +    do-bulk-command 1 = not IF

"= not" ==> "<>"

Or maybe you could even simply say:

  do-bulk-command not IF

... since the C functions are simply returning "bool"s - and the
IF-statement in Forth does not care whether the "true" value on the
stack is -1 or 1.

> +        ." USB-DISK: Bulk command failed!" cr
>          0 0 -1 EXIT
>      THEN
> +    dma-buf usb>data usb-buf-addr usb-buf-len move
>  
>      dma-buf usb>csw to csw-addr
>      csw-addr csw>sig l@ 55534253 <> IF
> 

The code looks indeed better the way you cleaned this up!

With or without the "= not" rework:

Reviewed-by: Thomas Huth <thuth at redhat.com>