Skip to content

Conversation

@N-R-K
Copy link
Contributor

@N-R-K N-R-K commented Oct 25, 2022

I noticed a couple things going on in set_alarm which seemed dubious. Here's a summary of the changes (additional context provided in commit message). Feel free to cherry-pick in case you don't agree with a certain commit.

  • value was unused, remove it.
  • value_type was set in attr but the necessary value_mask wasn't set on flags. So add XSyncCAValueType to flags.
  • Instead of destroying and creating alarm, just use XSyncChangeAlarm to change the existing alarm.
  • Ensure multiplication takes place in unsigned long instead of casting the result (otherwise that expression would always result in 0).

N-R-K added 4 commits October 25, 2022 14:17
value_type was being set in `attr` but it wasn't set in `flags`.
the cast here seems dobious. the multiplication will take place in
`unsigned int` and the cast only affects the _result_ and not the
multiplication itself. which means that the high 32bits will remain zero
and the shift will thus always produce zero as well.

use UL suffix on the integer constant to ensure that the multiplication
itself happens at `unsigned long`.
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