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

refactor: replace memset with a loop #2465

Merged
merged 1 commit into from
Dec 14, 2023

Replace memset(int32_t*, -1, _) with a for-loop

55a7600
Select commit
Loading
Failed to load commit list.
Merged

refactor: replace memset with a loop #2465

Replace memset(int32_t*, -1, _) with a for-loop
55a7600
Select commit
Loading
Failed to load commit list.
Mergeable / Mergeable succeeded Dec 14, 2023 in 5m 41s

2 checks passed!

Status: PASS

Details

✔️ Validator: TITLE

  • ✔️ All the requisite validations passed for 'or' option
    Input : refactor: replace memset with a loop
    Settings : {"or":[{"must_include":{"regex":"^(feat|docs|chore|cleanup|fix|refactor|test|style|perf)(\\(\\w+\\))?:\\ .+$","message":"Semantic release conventions must be followed."}},{"must_include":{"regex":"^Bump [^ ]* from [^ ]* to [^ ]*$","message":"Dependabot PRs are exempt from semantic release conventions."}}]}

✔️ Validator: DESCRIPTION

  • ✔️ description must exclude '\[ \]'
    Input : memset() treats the passed buffer as a char* array, assigning to every 1-byte of the array the value. So for a single 4-byte int32_t element, it is assigning bytes 0, 1, 2 and 3 of it to -1. It happens that -1 is 0xFF, so in the end the uint32_t is set to 0xFFFFFFFF, which is -1 in the two's complement, so the memset() actually produces the correct result in the end, assuming the platform uses two's complement integers.

Assigning it in the loop is less error-prone, as using memset() on non-1-byte wide arrays with a non-zero value is fishy, and it is more portable as we don't have to assume the use of two's complement.

On IRC, @robinlinden has pointed out that two's complement is the only integer format in C23 so perhaps we shouldn't be as concerned with the portability, but @iphy said that it's still a good idea to use a for-loop for this case.

It looks like in a future version of the C standard, C23, two's complement is the only integer format in C23 (thanks to @robinlinden on IRC for pointing that out), so perhaps we shouldn't be as concerned with the portability here? Though @iphydf says that it's still a good idea to use a for-loop for this case.


This change is Reviewable

Settings : {"must_exclude":{"regex":"\\\\[ \\\\]","message":"There are incomplete TODO task(s) unchecked."}}