cleanup: Small improvements found by PVS Studio.

[iphydf]

This change is 

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8201019) 73.11% compared to head (961891d) 73.02%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2666      +/-   ##
==========================================
- Coverage   73.11%   73.02%   -0.09%     
==========================================
  Files         148      148              
  Lines       30487    30482       -5     
==========================================
- Hits        22292    22261      -31     
- Misses       8195     8221      +26     

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

[nurupo]

Reviewed 5 of 6 files at r1, all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @iphydf)

---

toxcore/group_chats.c line 2601 at r1 (raw file):

{
    static_assert(sizeof(GC_Peer) < 2048, "GC_Peer is too large for stack allocation");
    GC_Peer self = {0};

Why is it stack-allocated now and we have the assert in case it the struct becomes too large... that feels like the step in the wrong direction, no?

Is it simply because the heap allocation is unnecessary in here since the object doesn't outlive the function scope, or is there a greater reason for this?

[JFreegman]

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @nurupo)

---

toxcore/group_chats.c line 2601 at r1 (raw file):

Previously, nurupo wrote…

Why is it stack-allocated now and we have the assert in case it the struct becomes too large... that feels like the step in the wrong direction, no?

Is it simply because the heap allocation is unnecessary in here since the object doesn't outlive the function scope, or is there a greater reason for this?

To add to this, what is the significance of 2048 bytes? I seem to recall that we try to keep stack allocations well below a certain threshold for musl builds but I can't remember what it is.

[iphydf]

2048 is half a memory page, so it was a nice round number. I could make the assert check for tox max packet size instead, which is what we frequently stack-allocate everywhere, but there's no relationship between peer struct size and packet size, other than that both can be stack allocated.

[nurupo]

Um. Kindly pointing out that my questions weren't rhetorical :D

[iphydf]

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @nurupo)

---

toxcore/group_chats.c line 2601 at r1 (raw file):

Previously, JFreegman wrote…

To add to this, what is the significance of 2048 bytes? I seem to recall that we try to keep stack allocations well below a certain threshold for musl builds but I can't remember what it is.

musl builds can't have stacks larger than 131072 bytes. So if we have a call stack 64 levels deep, and every level allocates 2KB, then we run out on musl.

Indeed it's just because the object doesn't outlive the function scope.

[nurupo]

Reviewed 2 of 2 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: complete! 1 of 1 approvals obtained