Skip to content

fix: Increase max group message length by four bytes#2351

Merged
JFreegman merged 1 commit into
TokTok:masterfrom
JFreegman:ngc_max_messge_length
Mar 3, 2023
Merged

fix: Increase max group message length by four bytes#2351
JFreegman merged 1 commit into
TokTok:masterfrom
JFreegman:ngc_max_messge_length

Conversation

@JFreegman
Copy link
Copy Markdown
Member

@JFreegman JFreegman commented Oct 12, 2022

The max message length was reduced by 4 bytes to account for the pseudo message ID, which had unintended effects on clients. It makes more sense to increase the raw packet length by four and keep the max group message length the same as the max message length for friend chats.


This change is Reviewable

@JFreegman JFreegman added bug Bug fix for the user, not a fix to a build script api break Change breaks API or ABI labels Oct 12, 2022
@JFreegman JFreegman added this to the v0.2.19 milestone Oct 12, 2022
@JFreegman JFreegman force-pushed the ngc_max_messge_length branch from 679598d to e8144a2 Compare October 12, 2022 17:53
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 12, 2022

Codecov Report

Merging #2351 (172f279) into master (b2ca401) will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #2351      +/-   ##
=========================================
- Coverage    9.94%   9.93%   -0.02%     
=========================================
  Files         141     141              
  Lines       32503   32503              
=========================================
- Hits         3234    3229       -5     
- Misses      29269   29274       +5     
Impacted Files Coverage Δ
auto_tests/tox_many_tcp_test.c 6.25% <0.00%> (-0.79%) ⬇️
toxcore/group_chats.c 7.21% <0.00%> (-0.14%) ⬇️
toxcore/TCP_client.c 5.89% <0.00%> (+0.22%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@zoff99
Copy link
Copy Markdown

zoff99 commented Oct 15, 2022

will this actually work? if you send a message with that new maximum size sending will fail, since GC_MESSAGE_PSEUDO_ID_SIZE bytes will be added.
send_gc_broadcast_message will fail then.

@JFreegman
Copy link
Copy Markdown
Member Author

send_gc_broadcast_message will fail then.

Why would it fail?

@JFreegman JFreegman closed this Mar 3, 2023
@JFreegman JFreegman force-pushed the ngc_max_messge_length branch from e8144a2 to b2ca401 Compare March 3, 2023 18:18
@JFreegman JFreegman reopened this Mar 3, 2023
@JFreegman JFreegman force-pushed the ngc_max_messge_length branch 4 times, most recently from c66e937 to c0002cc Compare March 3, 2023 20:53
The max message length was reduced by 4 bytes to account for the pseudo message ID, which had unintended effects on clients. It makes more sense to increase the raw packet length by four and keep the max group message length the same as the max message length for friend chats.
@JFreegman JFreegman force-pushed the ngc_max_messge_length branch from c0002cc to 172f279 Compare March 3, 2023 20:53
@JFreegman JFreegman merged commit 172f279 into TokTok:master Mar 3, 2023
@JFreegman JFreegman deleted the ngc_max_messge_length branch March 3, 2023 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api break Change breaks API or ABI bug Bug fix for the user, not a fix to a build script

Development

Successfully merging this pull request may close these issues.

3 participants