Skip to content
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

RangeIntPublisher add explicit cast to int #1443

Merged
merged 1 commit into from
Mar 16, 2021

Conversation

Scottmitch
Copy link
Member

Motivation:
RangeIntPublisher relies upon an implict case from long to int which is
gaurded by a min(int, long) (where the int is non-negative). CodeQL
highlights this as a warning.

Modifications:

  • Add an explicit cast to int to be more clear this is intentional

Result:
CodeQL warning is resovled.

Motivation:
RangeIntPublisher relies upon an implict case from long to int which is
gaurded by a `min(int, long)` (where the int is non-negative). CodeQL
highlights this as a warning.

Modifications:
- Add an explicit cast to int to be more clear this is intentional

Result:
CodeQL warning is resovled.
@Scottmitch
Copy link
Member Author

@Scottmitch
Copy link
Member Author

trivial change, will merge.

@Scottmitch Scottmitch merged commit ecb85bd into apple:main Mar 16, 2021
@Scottmitch Scottmitch deleted the range_explicit_cast branch March 16, 2021 23:03
bondolo pushed a commit to bondolo/servicetalk that referenced this pull request Mar 16, 2021
Motivation:
RangeIntPublisher relies upon an implict case from long to int which is
gaurded by a `min(int, long)` (where the int is non-negative). CodeQL
highlights this as a warning.

Modifications:
- Add an explicit cast to int to be more clear this is intentional

Result:
CodeQL warning is resovled.
@idelpivnitskiy
Copy link
Member

The change LGTM, but while I was looking at range, why we do not allow a negative stride? Python allows something like range(25, 2, -2): https://www.geeksforgeeks.org/python-range-function/

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.

2 participants