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

Add the missing HASH_REPLACE_KEYPTR #193

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

oakaigh
Copy link

@oakaigh oakaigh commented Jan 1, 2020

@troydhanson

HASH_ADD_KEYPTR is used when the structure contains a pointer to the key, rather than the key itself.

Note the inconsistency here. What if we need to replace the structure containing a pointer to the key?

@Quuxplusone
Copy link
Collaborator

Seems reasonable to me, but needs new tests for each of the newly added macros.

@oakaigh
Copy link
Author

oakaigh commented Jan 1, 2020

@Quuxplusone

needs new tests for each of the newly added macros.

Writing tests wouldn’t be trivial as long as you understand what this patch is doing. Essentially I’m just bridging &((add)->fieldname) with keyptr to make up the mess created by the original author. So no new test cases should be considered. All you have to do is to change the parameters passed to the function.

TL;DR

Honestly if you don’t want this project to look like a college startup project please merge this pull request as soon as possible. Also if you are the author please consider reorganizing your code base a little bit. Your code is not in standard format (why would you add extraneous braces (e.g. &((add)->fieldname)) around pointers) and this makes it extremely hard to read.

Quuxplusone added a commit to Quuxplusone/uthash that referenced this pull request Jul 11, 2023
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.

2 participants