Eric J. Bowersox wrote:
> I'm not sure why this happens, but the hash_lan function in ipmi_lan.c,
> when compiled in 64-bit mode, sometimes returns negative numbers, which
> results in a segfault inside the library. I suspect the games it's
> playing with pointer math aren't exactly 64-bit clean. Adding an
> operation to mask the index value to 31 bits after the shift and before
> the modulo operation clears this up. I doubt it will seriously affect
> the output of the hash function, since the mask operation is masking off
> bits that "should" be removed by the modulo operation anyway.
>
> This problem has existed at least as far back as OpenIPMI 2.0.10, where
> I first noticed it. When we recently upgraded the version of OpenIPMI
> we use to 2.0.14, this problem reared its ugly head again. Hopefully, I
> can get this fix in the mainline code and we'll never have to worry
> about it again.
>
> diff -Naur OpenIPMI-2.0.14-ORIGINAL/lib/ipmi_lan.c
> OpenIPMI-2.0.14/lib/ipmi_lan.c
> --- OpenIPMI-2.0.14-ORIGINAL/lib/ipmi_lan.c 2007-10-16
> 11:46:15.000000000 -0600
> +++ OpenIPMI-2.0.14/lib/ipmi_lan.c 2008-11-17 10:48:51.000000000
> -0700
> @@ -1352,6 +1352,8 @@
>
> idx = (((unsigned long) ipmi)
> >> (sizeof(unsigned long) >> LAN_HASH_SHIFT));
> + /* sometimes 64-bit pointer math gets in the way, mask to 31 bits
> */
> + idx &= 0x7FFFFFFF;
> idx %= LAN_HASH_SIZE;
> return idx;
> }
>
The following change is already checked in to fix that problem (and
another dealing with incorrect hashing). Thanks for the report and
patch, though.
-corey
Index: lib/ipmi_lan.c
===================================================================
RCS file: /cvsroot/openipmi/OpenIPMI/lib/ipmi_lan.c,v
retrieving revision 1.116
retrieving revision 1.117
diff -u -r1.116 -r1.117
--- lib/ipmi_lan.c 11 Apr 2008 05:26:04 -0000 1.116
+++ lib/ipmi_lan.c 7 Nov 2008 15:20:29 -0000 1.117
@@ -1349,13 +1349,13 @@
static lan_link_t lan_list[LAN_HASH_SIZE];
static lan_link_t lan_ip_list[LAN_HASH_SIZE];
-static int
+static unsigned int
hash_lan(const ipmi_con_t *ipmi)
{
- int idx;
+ unsigned int idx;
idx = (((unsigned long) ipmi)
- >> (sizeof(unsigned long) >> LAN_HASH_SHIFT));
+ >> (sizeof(unsigned long) + LAN_HASH_SHIFT));
idx %= LAN_HASH_SIZE;
return idx;
}
@@ -1392,7 +1392,7 @@
static void
lan_add_con(lan_data_t *lan)
{
- int idx;
+ unsigned int idx;
lan_link_t *head;
unsigned int i;
@@ -1441,8 +1441,8 @@
static lan_data_t *
lan_find_con(ipmi_con_t *ipmi)
{
- int idx;
- lan_link_t *l;
+ unsigned int idx;
+ lan_link_t *l;
ipmi_lock(lan_list_lock);
idx = hash_lan(ipmi);
@@ -5292,9 +5292,9 @@
static lan_data_t *
find_matching_lan(lan_conn_parms_t *cparm)
{
- lan_link_t *l;
- lan_data_t *lan;
- int idx;
+ lan_link_t *l;
+ lan_data_t *lan;
+ unsigned int idx;
/* Look in the first IP addresses list. */
idx = hash_lan_addr(&cparm->ip_addr[0].s_ipsock.s_addr);
@@ -5807,7 +5807,7 @@
lan_link_t *l;
lan_data_t *lan;
unsigned int i;
- int idx;
+ unsigned int idx;
lan_do_evt_t *found = NULL;
lan_do_evt_t *next = NULL;
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Openipmi-developer mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
