-
Notifications
You must be signed in to change notification settings - Fork 559
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 Serialization for khash map #76
base: master
Are you sure you want to change the base?
Conversation
I've updated this to work for sets as well as maps. |
It looks like this is writing bits to a file in a specific order, not "stringifying" the khash contents. (In case anyone else was wondering.) Also, why write to a filepath instead of a caller-supplied buffer? |
@@ -263,6 +309,30 @@ static inline int kputl(long c, kstring_t *s) | |||
return 0; | |||
} | |||
|
|||
|
|||
static inline int kputl_(long c, kstring_t *s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This duplicates a lot of code and it's not clear how it's related to the serialization.
kputl
could call kputl_
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unrelated to serialization. I added this so that I'd have the option of integer formatting routines which did not null-terminate for cases where I knew I would be appending to the string further. I can separate that out from this pull request later if requested. Compare kputw and kputw_ which were already present, for example. I'm not quite sure I understand why only some types were provided separate functions.
That's a reasonable point. I think having separate |
I've updated this. |
I've tested this on a map with 4096 keys, and kh_exist, kh_key, and kh_val all agreed between original and loaded maps. This only works, however, with maps, not sets.