Skip to content

Range checking and miscellaneous fixes tin time library #4466

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

Merged
merged 4 commits into from
Jan 23, 2013
Merged

Range checking and miscellaneous fixes tin time library #4466

merged 4 commits into from
Jan 23, 2013

Conversation

ScriptDevil
Copy link
Contributor

This primarily adds range-checking that was requested in #2350. This is done using a helper function match_digits_in_range.

%u takes a day of the week in the range of 1-7. However it was previously treated identically to %w that takes a value in the range 0-6. Similarly, %y takes only a two-digit year and thereby subtracting 1900 from it is wrong. This is also fixed.

Further, I have also fixed the error messages in a few places where it was wrong.

%u flag takes a value in the range of 1-7. However value needs to be
stored in tm.tm_wday between 0 and 6.
%y takes a two-digit year value. Subtracting 1900_i32 from it is not
needed.
This was requested in Issue #2350. New function match_digits_in_range
added and used instead of match_digits wherever needed.
@catamorphism
Copy link
Contributor

Thanks for fixing this! If you run make check and fix any errors that make tidy reports, I'll merge.

@catamorphism
Copy link
Contributor

Sorry for the delay in merging, I didn't see the long-lines fix right away. Thanks!

catamorphism added a commit that referenced this pull request Jan 23, 2013
Range checking and miscellaneous fixes tin time library
@catamorphism catamorphism merged commit 1a226f5 into rust-lang:incoming Jan 23, 2013
@graydon
Copy link
Contributor

graydon commented Jan 23, 2013

Greatly appreciate the fix, but in the future if you could run make check, existing unit tests often find overlooked bugs. There was a small asymmetry between this change and the existing behavior of strftime that broke such a test. Fixed in 93e969e.

@catamorphism
Copy link
Contributor

Sorry for getting overly enthusiastic with merging the pull requests! I should have run make check first.

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.

3 participants