Hi!

I just wonder how safe the code is:

Doesn't the difference of two unsigned ints give an unsigned value? The 
assigning an unsigned int to a signed int will definitely reduce the range...


I feel that 
s32 diff = (s32) n1 - (s32) n2; 

also doesn't make the problem go away, unless ypou promote an unsigned 32 bit 
value to a signed 64 bit value. If you are really thinking in assembler, you 
should add comments on that. The code looks dubious to me at least.

Regards,
Ulrich


>>> Mark Rustad <mark.d.rus...@intel.com> schrieb am 07.04.2011 um 23:26 in
Nachricht <20110407212615.20399.18029.stgit@localhost6.localdomain6>:
> Unsigned serial number comparison is very simple if you simply put the
> difference into a signed integer of the same size and then compare that
> value with zero. All the complexity and confusion fall away.
> 
> Signed-off-by: Mark Rustad <mark.d.rus...@intel.com>
> ---
>  include/scsi/iscsi_proto.h |   21 ++++++++++++---------
>  1 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/include/scsi/iscsi_proto.h b/include/scsi/iscsi_proto.h
> index 0c6c1d6..98ebc3d 100644
> --- a/include/scsi/iscsi_proto.h
> +++ b/include/scsi/iscsi_proto.h
> @@ -35,30 +35,33 @@
>  /*
>   * Serial Number Arithmetic, 32 bits, RFC1982
>   */
> -#define SNA32_CHECK 2147483648UL
>  
>  static inline int iscsi_sna_lt(u32 n1, u32 n2)
>  {
> -     return n1 != n2 && ((n1 < n2 && (n2 - n1 < SNA32_CHECK)) ||
> -                     (n1 > n2 && (n2 - n1 < SNA32_CHECK)));
> +     s32 diff = n1 - n2;
> +
> +     return diff < 0;
>  }
>  
>  static inline int iscsi_sna_lte(u32 n1, u32 n2)
>  {
> -     return n1 == n2 || ((n1 < n2 && (n2 - n1 < SNA32_CHECK)) ||
> -                     (n1 > n2 && (n2 - n1 < SNA32_CHECK)));
> +     s32 diff = n1 - n2;
> +
> +     return diff <= 0;
>  }
>  
>  static inline int iscsi_sna_gt(u32 n1, u32 n2)
>  {
> -     return n1 != n2 && (((n1 < n2) && ((n2 - n1) > SNA32_CHECK)) ||
> -                     ((n1 > n2) && ((n1 - n2) < SNA32_CHECK)));
> +     s32 diff = n1 - n2;
> +
> +     return diff > 0;
>  }
>  
>  static inline int iscsi_sna_gte(u32 n1, u32 n2)
>  {
> -     return n1 == n2 || (((n1 < n2) && ((n2 - n1) > SNA32_CHECK)) ||
> -                     ((n1 > n2) && ((n1 - n2) < SNA32_CHECK)));
> +     s32 diff = n1 - n2;
> +
> +     return diff >= 0;
>  }
>  
>  /*



 

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com.
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/open-iscsi?hl=en.

Reply via email to