Skip to content
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.

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

ofi_list: Add dlist_size and slist_size #9360

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sunkuamzn
Copy link
Contributor

Useful for printing the size of the list during debugging

@sunkuamzn sunkuamzn force-pushed the dlist-size branch 2 times, most recently from 75e6580 to ef073ff Compare October 10, 2023 15:34
@sunkuamzn
Copy link
Contributor Author

AWS CI failures are static assert failures. I will fix them.

Copy link
Member

@shefty shefty left a comment

Choose a reason for hiding this comment

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

Thanks! I think this will be very useful for debugging. See comments for changes

include/ofi_list.h Show resolved Hide resolved
include/ofi_list.h Show resolved Hide resolved
@@ -388,11 +388,13 @@ struct slist_entry {
struct slist {
struct slist_entry *head;
struct slist_entry *tail;
OFI_DBG_VAR(size_t, size);
Copy link
Member

Choose a reason for hiding this comment

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

Because of how this macro works, we can't add the ';' at the end of the line. Some compilers don't like that.

static inline size_t slist_size(struct slist *list)
{
return list->size;
}
Copy link
Member

Choose a reason for hiding this comment

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

Please add a non-debug version of this function that returns 0, so apps can just use it (e.g. in a log message) without guarding the code with debug checks.

We could consider changing the return type to ssize_t and return -1 in the free build as well, to better indicate that the value isn't known.

@@ -57,15 +57,23 @@ enum ofi_list_end {
struct dlist_entry {
struct dlist_entry *next;
struct dlist_entry *prev;
OFI_DBG_VAR(size_t, size);
OFI_DBG_VAR(struct dlist_entry *, list);
Copy link
Member

Choose a reason for hiding this comment

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

Remove ';' from end of the OFI_DBG_VAR() macro lines

include/ofi_list.h Show resolved Hide resolved
include/ofi_list.h Show resolved Hide resolved
@sunkuamzn
Copy link
Contributor Author

I won't be able to work on this for the next few weeks

Signed-off-by: Sai Sunku <sunkusa@amazon.com>
Signed-off-by: Sai Sunku <sunkusa@amazon.com>
Copy link
Member

@shefty shefty left a comment

Choose a reason for hiding this comment

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

thanks! no idea about the CI failures, but the changes look good to me.

@@ -435,8 +439,10 @@ static inline struct slist_entry *slist_remove_head(struct slist *list)
item = list->head;
if (list->head == list->tail)
slist_init(list);
else
else {
Copy link
Member

Choose a reason for hiding this comment

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

nit: Please add braces to the if side if needed on the else side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants