Skip to content

fix: Remove fatal error for non-erroneous case#2555

Merged
Green-Sky merged 1 commit intoTokTok:masterfrom
JFreegman:group_null_data
Jan 13, 2024
Merged

fix: Remove fatal error for non-erroneous case#2555
Green-Sky merged 1 commit intoTokTok:masterfrom
JFreegman:group_null_data

Conversation

@JFreegman
Copy link
Copy Markdown
Member

@JFreegman JFreegman commented Jan 12, 2024

We allow non-null data pointers to be passed to functions alongside 0-length data. For example when creating a data buffer that has room for the entire packet, including ignored header data.

This error broke a rare but legitimate case where we miss packets during a handshake attempt and need to store empty handshake packets in the packet array.


This change is Reviewable

@JFreegman JFreegman added the bug Bug fix for the user, not a fix to a build script label Jan 12, 2024
@JFreegman JFreegman added this to the v0.2.19 milestone Jan 12, 2024
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 12, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (812f931) 68.27% compared to head (07fe665) 68.29%.

Files Patch % Lines
toxcore/group_connection.c 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2555      +/-   ##
==========================================
+ Coverage   68.27%   68.29%   +0.01%     
==========================================
  Files         118      118              
  Lines       28162    28162              
==========================================
+ Hits        19227    19232       +5     
+ Misses       8935     8930       -5     

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

@JFreegman JFreegman force-pushed the group_null_data branch 2 times, most recently from 6ecb9a6 to 6d0dbb5 Compare January 12, 2024 17:44
commit 5b9c420 introduced some undesirable behaviour with packet send
functions returning error when they shouldn't. We now only return an
error if the packet fails to be added to the send queue or cannot
be wrapped/encrypted. We no longer error if we fail to send the packet
over the wire, because toxcore will keep trying to re-send the packet
until the connection times out.

Additionally, we now make sure that our packet broadcast functions
aren't returning an error when failing to send packets to peers
that we have not successfully handshaked with yet, since this is
expected behaviour.
Copy link
Copy Markdown
Member

@Green-Sky Green-Sky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still dont thing zero sized arrays should be non-null, but I want master working again.

@Green-Sky Green-Sky merged commit 07fe665 into TokTok:master Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug fix for the user, not a fix to a build script

Development

Successfully merging this pull request may close these issues.

2 participants