[Asterisk-code-review] Astobj2: Correctly treat hash fn returning INT MIN (asterisk[11])

[Corey Farrell]

Corey Farrell has posted comments on this change.

Change subject: Astobj2: Correctly treat hash_fn returning INT_MIN
......................................................................


Patch Set 2: Code-Review-1

(2 comments)

My comment is purely about clarity of the code.  This does change the final result for negative values returned by c->hash_fn, so someone else will need to approve the algorithm update.

https://gerrit.asterisk.org/#/c/532/2/main/astobj2.c
File main/astobj2.c:

Line 903: 	i = (unsigned int)c->hash_fn(user_data, OBJ_POINTER) % c->n_buckets;
I don't like that we're casting to unsigned int, performing modulus with a signed int, then assigning to a signed int.

As an alternative, what about:
i = abs(c->hash_fn(user_data, OBJ_POINTER) % c->n_buckets);

The modulus will always bring the value closer to 0, so it would be impossible for us to run abs(INT_MIN).  Everything operates on signed int, no casts.


Line 1067: 		start = i = (unsigned int)c->hash_fn(arg, flags & (OBJ_POINTER | OBJ_KEY)) % c->n_buckets;
Same here.


-- 
To view, visit https://gerrit.asterisk.org/532
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id6981400ad526f47e10bcf7b847b62bd2785e899
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Ivan Poddubny <ivan.poddubny at gmail.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-HasComments: Yes