Skip to content

Conversation

joriskleiber
Copy link

This pull request replaces system::time::Duration with std::time::Duration.

I couldn't find any code that still uses negative duration, so I replaced it with it's std equivalent.
The only place, where I'm not sure if its possible to use an negative duration, is the prior_validity property from the Authentication struct.

Should close #1170

pwfeedback: self.pwfeedback(),
password_timeout: match self.passwd_timeout() {
password_timeout: match self.passwd_timeout().try_into().unwrap() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no validation in fractional_minutes that no negative numbers are used by the user, so this unwrap can trigger.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it, so it uses the default value of passwd_timeout when triggering the unwrap. Is this also a problem for prior_validity?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should already fail in the config parser.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too familiar with the code, what is the config parser and should only passwd_timeout fail when negative?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fn fractional_minutes(input: &str) -> Option<i64> {

@joriskleiber joriskleiber force-pushed the replace-duration branch 2 times, most recently from 562b268 to 9f08633 Compare July 24, 2025 20:49
@squell squell added the freeze Temporarily blocked from merging pending a release label Jul 28, 2025
@squell squell removed the freeze Temporarily blocked from merging pending a release label Aug 4, 2025
@joriskleiber joriskleiber requested a review from bjorn3 August 7, 2025 11:58
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.

Investigate replacing system::time::Duration
3 participants