[GitHub] [geode] sabbey37 commented on a change in pull request #6588: GEODE-9366: Redis SynchronizedStripeExecutor can throw IndexOutOfBoundsException

[GitBox <gi...@apache.org>]

sabbey37 commented on a change in pull request #6588:
URL: https://github.com/apache/geode/pull/6588#discussion_r647683217



##########
File path: geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/SynchronizedStripedExecutor.java
##########
@@ -59,10 +59,13 @@ private Object getSync(Object stripeId) {
 
   private int getStripeIndex(Object stripeId) {
     int hash = stripeId.hashCode();
+
+    hash %= syncs.length;
     if (hash < 0) {
       hash = -hash;

Review comment:
       Apologies if this is obvious and I've forgotten, but is there a reason we don't do `Math.abs(hash)` here?  Also, it's interesting the way `Math.abs()` handles the `INTEGER.MIN_VALUE`:
   ```
   Note that if the argument is equal to the value of Integer.MIN_VALUE, the most negative representable int value, the result is that same value, which is negative.
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org