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.
Is there any problem with this implementation that I don't see? Are there
architectures that will mishandle assigning an unsigned difference to a signed?
I have used this method to fix several cases of broken sequence count
comparisons in a variety of contexts over the years, but this is the first time
for iscsi.
--
Mark Rustad, LAN Access Division, Intel Corporation
--
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.
