Skip to content

Fix wrong allocation size of ringbuffer #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 21, 2021
Merged

Fix wrong allocation size of ringbuffer #3

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions ringbuffer/ringbuffer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,9 @@ typedef struct {
/* true if x is a power of 2 */
#define IS_POWEROF2(x) ((((x) -1) & (x)) == 0)
#define RING_SIZE_MASK (unsigned) (0x0fffffff) /**< Ring size mask */
#define ALIGN_FLOOR(val, align) \
(typeof(val))((val) & (~((typeof(val))((align) -1))))
#define ALIGN_CEIL(val, align) \
(typeof(val))((val) + (-(typeof(val))(val) & ((align) -1)))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check if this is what you want, thanks!



/**
* Calculate the memory size needed for a ring buffer.
Expand All @@ -80,7 +81,7 @@ ssize_t ringbuf_get_memsize(const unsigned count)
return -EINVAL;

ssize_t sz = sizeof(ringbuf_t) + count * sizeof(void *);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The right way is to mention the size, which is multiple of cache line size.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be alright to limit the size to the multiple of cache line size. But in this situation, I think ALIGN_FLOOR won't change anything of the original sz. Is there any reason to use ALIGN_FLOOR here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RinHizakura, you are right. ALIGN_CEIL should be used for allocating dynamic memory. I was thinking of the memory pool which manages the elements where ALIGN_FLOOR was used.

sz = ALIGN_FLOOR(sz, CACHE_LINE_SIZE);
sz = ALIGN_CEIL(sz, CACHE_LINE_SIZE);
return sz;
}

Expand Down