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

feat: Implement support for multiple 'H' ranges in one field #3

Merged
merged 14 commits into from
Oct 22, 2024

Conversation

Kniggebrot
Copy link

If no range is specified, they will return the same number.
But if different ranges (optionally with different offsets) are specified, different numbers should be returned due to the different modulo and/or offsets of the ranges.
Thus, multiple 'H' with ranges in one field should be allowed for convenience.

If no range is specified, they will return the same number.
But if different ranges (optionally with different offsets) are specified, different numbers should be returned due to the different modulo and/or offsets of the ranges.
Thus, multiple 'H' with ranges in one field should be allowed for convenience.
Easier to see which test failed
Useful for putting the parsed H fields back together
The issue was the missed `free()` of the result fields being merged into the final field.
As the original subfields are already freed in `replace_hashed()`, this should now happen in the final free for the list subfields.
Also removed the `str_append()` utility, as allocating one big string and filling it with the single results (via sprintf) may take more runtime (for looping twice), but simplifies memory management and improves readability.
Contents are identical to subfields. Can simply use it instead
It requires tracking of the substring lens, but it should be more efficient with shorter writes.
Regarding parsability, combinability with other options
Add commas when needed. Also use strcpy and a tracking pointer to make copying more efficient
Free the accum_field and subfields always if an error occurs during field replacement (in a list) and they were malloced
@Kniggebrot Kniggebrot merged commit 41199d9 into master Oct 22, 2024
1 check passed
@Kniggebrot Kniggebrot deleted the feature/multiple-h branch October 22, 2024 11:18
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.

1 participant