Skip to content

Conversation

@pabigot
Copy link
Contributor

@pabigot pabigot commented Oct 3, 2019

Legacy code uses integer literal delays measured in milliseconds as arguments to kernel API that is intended to take timeout values. Replace these integers with the corresponding defined constants and macro conversions that represent the timeout values.

This is a demonstration and prototype of using semantic patch technology to repeatably perform the conversions necessary to support #17155, done in a way that is reviewable, verifiable, and easy to re-apply when pending work is merged to master or rebases are required.

@zephyrbot zephyrbot added area: Sensors Sensors area: ADC Analog-to-Digital Converter (ADC) area: POSIX POSIX API Library area: Networking area: Logging area: Shell Shell subsystem area: Bluetooth area: Samples Samples labels Oct 3, 2019
@zephyrbot zephyrbot added area: Tests Issues related to a particular existing or missing test area: Kernel labels Oct 3, 2019
@zephyrbot
Copy link

zephyrbot commented Oct 3, 2019

All checks are passing now.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@andyross
Copy link
Contributor

andyross commented Oct 3, 2019

No complaints here beyond whines about a bunch of extra work, I guess. Is the intent to get this in first and then have me rebase all the collisions in #17155 on top?

@pabigot pabigot force-pushed the dnm/cocci-timeout branch from 87d2bf2 to 211125d Compare October 3, 2019 07:58
@pabigot
Copy link
Contributor Author

pabigot commented Oct 3, 2019

No complaints here beyond whines about a bunch of extra work, I guess. Is the intent to get this in first and then have me rebase all the collisions in #17155 on top?

Short answer: no, the intent is to get this in, then revisit the goals of #17155 and proceed in a more sustainable manner. The longer answer is here.

Copy link
Member

Choose a reason for hiding this comment

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

This is calling a libc API for the native posix build (i.e. the host libc), so this change is wrong (it will break once the macro expands to something else than an integer).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Mis-placed parentheses in the regular expression caused the conversion to affect poll instead of k_poll. Fixed now.

Some legacy code still passes integer literals in milliseconds as the
value to functions that take a timeout.  This usage interferes with
plans to replace the millisecond representation with a more generic
k_timeout_t value.  Add a Coccinelle script to convert call sites to
use the proper constants and macros.

Signed-off-by: Peter Bigot <[email protected]>
Use the int_literal_to_timeout Coccinelle script to convert literal
integer arguments for kernel API timeout parameters to the standard
timeout value representations.

Signed-off-by: Peter Bigot <[email protected]>
@pabigot pabigot force-pushed the dnm/cocci-timeout branch from 211125d to b040b66 Compare October 3, 2019 11:23
@pabigot
Copy link
Contributor Author

pabigot commented Oct 3, 2019

What's the process for removing the shippable override patch without triggering CI again, which will then fail?

@galak
Copy link
Contributor

galak commented Oct 3, 2019

What's the process for removing the shippable override patch without triggering CI again, which will then fail?

  1. remove the commit
  2. ask a maintainer to force merge

@pabigot pabigot force-pushed the dnm/cocci-timeout branch from 3548645 to b040b66 Compare October 3, 2019 18:50
@pabigot
Copy link
Contributor Author

pabigot commented Oct 3, 2019

@galak Thanks, done. Looks like history of the success has disappeared, but it was https://app.shippable.com/github/zephyrproject-rtos/zephyr/runs/52375/summary/console.

@galak galak merged commit ab91eef into zephyrproject-rtos:master Oct 3, 2019
@galak
Copy link
Contributor

galak commented Oct 3, 2019

@galak Thanks, done. Looks like history of the success has disappeared, but it was https://app.shippable.com/github/zephyrproject-rtos/zephyr/runs/52375/summary/console.

merged

@pabigot pabigot deleted the dnm/cocci-timeout branch October 5, 2019 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ADC Analog-to-Digital Converter (ADC) area: Bluetooth area: Kernel area: Logging area: Networking area: POSIX POSIX API Library area: Samples Samples area: Sensors Sensors area: Shell Shell subsystem area: Tests Issues related to a particular existing or missing test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants