Skip to content

Conversation

iphydf
Copy link
Member

@iphydf iphydf commented Jul 13, 2025

Also add a bunch of casts where needed. I've tried to model everything in such a way that it minimises casts. The casts should be safe, but it's not always obvious. In the obvious cases, we should have a linter that validates it. In the non-obvious cases, that linter should warn and require that we add a null check. I've added some null checks in some cases but not all.

Also, refactored some of the constructor functions to never assign a maybe-null value to a non-null struct member, instead using a temporary local variable to check if construction/allocation succeeded.


This change is Reviewable

@iphydf iphydf added this to the v0.2.22 milestone Jul 13, 2025
@github-actions github-actions bot added the refactor Refactoring production code, eg. renaming a variable, not affecting semantics label Jul 13, 2025
@iphydf iphydf force-pushed the struct-nulls branch 3 times, most recently from b654efa to c023b7b Compare July 13, 2025 19:59
Copy link

codecov bot commented Jul 13, 2025

Codecov Report

Attention: Patch coverage is 79.69432% with 93 lines in your changes missing coverage. Please review.

Project coverage is 72.13%. Comparing base (3e22fd5) to head (c023b7b).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
toxcore/TCP_connection.c 45.00% 44 Missing ⚠️
toxcore/tox.c 78.08% 16 Missing ⚠️
toxcore/group_chats.c 47.82% 12 Missing ⚠️
toxcore/announce.c 27.27% 8 Missing ⚠️
toxcore/group_connection.c 78.57% 3 Missing ⚠️
toxcore/friend_connection.c 66.66% 2 Missing ⚠️
toxcore/group_moderation.c 77.77% 2 Missing ⚠️
toxcore/tox_events.c 80.00% 2 Missing ⚠️
toxcore/TCP_server.c 87.50% 1 Missing ⚠️
toxcore/net_crypto.c 92.85% 1 Missing ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2914      +/-   ##
==========================================
+ Coverage   72.07%   72.13%   +0.05%     
==========================================
  Files         154      154              
  Lines       31272    31396     +124     
==========================================
+ Hits        22540    22646     +106     
- Misses       8732     8750      +18     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@iphydf iphydf marked this pull request as ready for review July 14, 2025 09:26
@iphydf iphydf force-pushed the struct-nulls branch 3 times, most recently from ddd160d to 4265c7c Compare July 14, 2025 21:22
@iphydf iphydf force-pushed the struct-nulls branch 2 times, most recently from 5c420c0 to 499f9ae Compare July 19, 2025 09:04
Also add a bunch of casts where needed. I've tried to model everything
in such a way that it minimises casts. The casts *should* be safe, but
it's not always obvious. In the obvious cases, we should have a linter
that validates it. In the non-obvious cases, that linter should warn and
require that we add a null check. I've added some null checks in some
cases but not all.

Also, refactored some of the constructor functions to never assign a
maybe-null value to a non-null struct member, instead using a temporary
local variable to check if construction/allocation succeeded.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactoring production code, eg. renaming a variable, not affecting semantics
Development

Successfully merging this pull request may close these issues.

2 participants