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

Auto free on zlists should use strcmp? #2204

Open
sphaero opened this issue Dec 10, 2021 · 4 comments
Open

Auto free on zlists should use strcmp? #2204

sphaero opened this issue Dec 10, 2021 · 4 comments

Comments

@sphaero
Copy link
Contributor

sphaero commented Dec 10, 2021

If you are using zlist_autofree (usually for storing strings) is there any situation imaginable where you don't want to use strcmp as a comparator function?

As zlist_autofree duplicates strings you can't use zlist_exists or zlist_remove if you don't set a comparator.

So why not set the comparator automatically?

zlist_comparefn (mylist, (zlist_compare_fn *) strcmp);

@bluca
Copy link
Member

bluca commented Dec 10, 2021

backward compat, probably?

@sphaero
Copy link
Contributor Author

sphaero commented Dec 11, 2021

I don't that would be an issue.
If you use zlist_autofree char pointers are implied:

czmq/src/zlist.c

Lines 183 to 186 in 062aeb3

if (self->autofree) {
item = strdup ((char *) item);
assert (item);
}

Using it like that renders the zlist_remove and zlist_exists useless as they compare pointers by default which is of no use as the strings are duped.

So if people are using zlist_autofree they need to set a compare function if they want any useful functionality. We can at least set it to a sane default. If people want something different they would already set so.

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity for 90 days. It will be closed if no further activity occurs within 21 days. Thank you for your contributions.

@stale stale bot added the stale label Apr 16, 2022
@sphaero
Copy link
Contributor Author

sphaero commented May 19, 2022

I still think this is a proper user friendly suggestion

@stale stale bot removed the stale label May 19, 2022
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

No branches or pull requests

2 participants