On Mar 3, 2011, at 4:27 PM, Rustad, Mark D wrote:
> Folks,
>
> I noticed some code in libiscsi used for comparing serial numbers that I
> think can be improved. Specifically this:
>
> /* Serial Number Arithmetic, 32 bits, less than, RFC1982 */
> #define SNA32_CHECK 2147483648UL
>
> static int iscsi_sna_lt(u32 n1, u32 n2)
> {
> return n1 != n2 && ((n1 < n2 && (n2 - n1 < SNA32_CHECK)) ||
> (n1 > n2 && (n2 - n1 < SNA32_CHECK)));
> }
>
> /* Serial Number Arithmetic, 32 bits, less than, RFC1982 */
> static int iscsi_sna_lte(u32 n1, u32 n2)
> {
> return n1 == n2 || ((n1 < n2 && (n2 - n1 < SNA32_CHECK)) ||
> (n1 > n2 && (n2 - n1 < SNA32_CHECK)));
> }
>
> I think can better be done as:
>
> /* Serial Number Arithmetic, 32 bits, less than, RFC1982 */
> static int iscsi_sna_lt(u32 n1, u32 n2)
> {
> s32 diff = n1 - n2;
>
> return diff < 0;
> }
>
> /* Serial Number Arithmetic, 32 bits, less than, RFC1982 */
> static int iscsi_sna_lte(u32 n1, u32 n2)
> {
> s32 diff = n1 - n2;
>
> return diff <= 0;
> }
>
> I think that this is both much clearer and should generate better code. I
> have run a test comparing the two implementations, and they only differ on
> numbers that are 2^31 apart, i.e. 0 and 0x80000000 and other pairs similarly
> spaced. According to RFC1982, these cases are arbitrary anyway.
Makes sense to me. FWWI, RFC 1982 is wrong when it discusses that case: it
claims that avoiding this indeterminacy would have the drawback that x < y does
not necessarily imply x+1 < y+1. But that's clearly not true, and its own
examples make that obvious.
paul
--
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/open-iscsi?hl=en.
