refactor: replace memset with a loop

[nurupo]

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 

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6645343) 74.51% compared to head (55a7600) 71.60%.
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2465      +/-   ##
==========================================
- Coverage   74.51%   71.60%   -2.92%     
==========================================
  Files          87       75      -12     
  Lines       26313    25189    -1124     
==========================================
- Hits        19607    18036    -1571     
- Misses       6706     7153     +447     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Green-Sky]

Using a simple for-loop is trivial for a compiler to optimize. I would not be surprised if the compiler uses memset or similar in optimized binaries from this code.

[robinlinden]

For the record, I'm pro for-loop and portability. My comments on IRC were only about the fact that C++20 and C23 require two's complement for signed integers and unrelated to this change.

[JFreegman]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: 0 of 1 approvals obtained (waiting on @nurupo)

a discussion (no related file):
I'm fine with this change but I think in the future we need to properly define what we consider to be portable-enough code for our purposes. There are probably many obscure systems that Tox would never run on regardless of an isolated improvement like this one, making these sorts of PR's effectively pointless in the grander scheme of things. What systems that people might use Tox on don't actually use two's compliment that we're making an active effort to support? We should have the CI run tests on all the systems that we want to actively support. We don't have to require them to pass at first, but the fact that they exist and give us some sort of a tangible goal would be very helpful.

Also, could you squash the bootstrap hash commit?

---

[nurupo]

@JFreegman The portability is one half of it, the other is that using memset() on non-1-byte wide arrays with a non-zero value is rather suspicious, as the programmer might have assumed that the value is assigned to every int32_t instead of every char. (This is a bit far fetched, but a person refactoring the code might read it as such too, e.g. mistakenly setting value to -2 thinking that it would work that way, assuming a hypothetical refactor requires -2 initializing the array). Now that I thought about it some more, if it was ~0 instead of -1, i.e. "all bits set", then the intention would be clearer.

[nurupo]

Squashed.

[nurupo]

@robinlinden reworded the commit message, hopefully you like it better now. Also @'ed the correct iphy. Sorry @iphy for the email spam.

[JFreegman]

Reviewed all commit messages.
Reviewable status: complete! 3 of 1 approvals obtained (waiting on @nurupo)