[GitHub] [flink] StefanRRichter commented on a change in pull request #8613: [FLINK-12730][Runtime / Coordination] Combine BitSet implementations in flink-runtime

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

StefanRRichter commented on a change in pull request #8613: [FLINK-12730][Runtime / Coordination] Combine BitSet implementations in flink-runtime
URL: https://github.com/apache/flink/pull/8613#discussion_r300622930
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/operators/util/BitSet.java
 ##########
 @@ -89,8 +88,13 @@ public int bitSize() {
 	 * Clear the bit set.
 	 */
 	public void clear() {
-		for (int i = 0; i < byteLength; i++) {
-			memorySegment.put(offset + i, (byte) 0);
+		int index = 0;
+		while (index < byteLength) {
 
 Review comment:
   This looks like a potential bug to me, assuming that we can always still add 8 bytes if we are smaller than `byteLength` can go out of bounds IMO. Or in case that it could not go out of bounds because it is always a multiple of 8, the second loop that follows does not make sense and it is currently also never invoked. I would then suggest to add a test that covers clearing from a bitset with an amount of bits that is not divisible by 8.

----------------------------------------------------------------
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


With regards,
Apache Git Services