forked from staticlibs/ccronexpr
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.