Bug: loop counter in ringbuf.h function __store_ring() and __load_ring() is wrong

[liuqun]

Both __store_ring() and __load_ring() have a same bug.

function __store_ring: https://github.com/opencoff/portable-lib/blob/10c32868e57c65b314e24834be6c62b1e856e74e/inc/fast/ringbuf.h#L237-L277

• Bug: When n=1, function __store_ring(ring, head, elements, n) actually writes 4 elements into the ring.

• Reason: loop should = n>>2 not n&0x03 Buggy code is at line 240: https://github.com/opencoff/portable-lib/blob/10c32868e57c65b314e24834be6c62b1e856e74e/inc/fast/ringbuf.h#L240

• Solution: The correct logic should be:

unsigned loop = n & ((unsigned) ~0x3U);

see also https://github.com/DPDK/dpdk/blob/07f08a96f5255a0375dce2683db4fb489bbaa8a0/lib/librte_ring/rte_ring.h#L240-L250

[opencoff]

Thank you for finding this issue. I patched it - but slightly different; I moved the unrolled-loop-count to inside the "am I wrapped or not" if () block. Please take a look at the latest commit.

Best,

刘群 wrote on 8/10/19 8:19 AM:

https://github.com/opencoff/portable-lib/blob/10c32868e57c65b314e24834be6c62b1e856e74e/inc/fast/ringbuf.h#L237-L277

Bug: When |n=1|, function |__store_ring(ring, head, elements, n)|
actually writes 4 elements into the ring.

Reason: |loop| should = |n>>2| not |n&0x03|
Buggy code is at line 240:
https://github.com/opencoff/portable-lib/blob/10c32868e57c65b314e24834be6c62b1e856e74e/inc/fast/ringbuf.h#L240

Solution:
The correct logic should let |loop = n % 4| or using bit-shift
operator '>>':

|unsigned loop = n >> 2; |

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/opencoff/portable-lib/issues/1?email_source=notifications&email_token=ACB7AW3MNRZXRQNKEDMMRSLQDZTWPA5CNFSM4IKYZZ4KYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HEQTEKQ, or mute the thread https://github.com/notifications/unsubscribe-auth/ACB7AWZXAEEPPYRV43EEAO3QDZTWPANCNFSM4IKYZZ4A.

[liuqun]

Fixed by 9b479750f6fb4f4265782370e238e4443cf22c93