Skip to content

Do not send the same packet to the same node twice#1310

Merged
robinlinden merged 1 commit intoTokTok:masterfrom
kurnevsky:fix_route_to_friend
May 1, 2019
Merged

Do not send the same packet to the same node twice#1310
robinlinden merged 1 commit intoTokTok:masterfrom
kurnevsky:fix_route_to_friend

Conversation

@kurnevsky
Copy link

@kurnevsky kurnevsky commented Feb 19, 2019

It's a bug introduced in c-toxcore after assocs refactoring. See the original code: https://github.com/irungentoo/toxcore/blob/bf69b54f64003d160d759068f4816b2d9b2e1e21/toxcore/DHT.c#L1740-L1763


This change is Reviewable

@zoff99
Copy link

zoff99 commented Feb 20, 2019

@kurnevsky are you sure this is correct?

@kurnevsky
Copy link
Author

@zoff99 I think yes, friend_sent in the current code does nothing while it's supposed to prevent sending packets to same nodes (check the original code). Now it can be done with a simple break since we have inner loop by node's addresses.

@zoff99
Copy link

zoff99 commented Feb 20, 2019

sorry did not see the inner loop. damn folding :-)

line 2004 - 2007 does not do anything even now. thats the issue then i guess. alright

Copy link

@sudden6 sudden6 left a comment

Choose a reason for hiding this comment

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

:lgtm_strong:

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained

@sudden6 sudden6 force-pushed the fix_route_to_friend branch from e4c238a to ec9dedf Compare April 25, 2019 20:23
@codecov
Copy link

codecov bot commented Apr 25, 2019

Codecov Report

Merging #1310 into master will decrease coverage by 1.1%.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #1310     +/-   ##
========================================
- Coverage    84.1%   82.9%   -1.2%     
========================================
  Files          86      82      -4     
  Lines       15708   14975    -733     
========================================
- Hits        13218   12429    -789     
- Misses       2490    2546     +56
Impacted Files Coverage Δ
toxcore/DHT.c 77.7% <100%> (ø) ⬆️
toxcore/group.c 78.8% <0%> (-13.1%) ⬇️
toxcore/state.c 66.6% <0%> (-7.4%) ⬇️
toxcore/tox_api.c 61.2% <0%> (-3.6%) ⬇️
toxcore/TCP_server.c 80.3% <0%> (-2.4%) ⬇️
toxcore/LAN_discovery.c 84.9% <0%> (-1.9%) ⬇️
toxencryptsave/toxencryptsave.c 70.2% <0%> (-1.7%) ⬇️
toxcore/friend_connection.c 94.9% <0%> (-1.1%) ⬇️
toxav/toxav.c 66.6% <0%> (-1%) ⬇️
toxcore/util.c 60% <0%> (-0.9%) ⬇️
... and 57 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b3100c9...d26b11d. Read the comment docs.

@robinlinden robinlinden force-pushed the fix_route_to_friend branch from ec9dedf to d26b11d Compare May 1, 2019 14:40
@robinlinden robinlinden merged commit d26b11d into TokTok:master May 1, 2019
@robinlinden robinlinden added this to the v0.2.x milestone May 1, 2019
@robinlinden robinlinden modified the milestones: v0.2.x, v0.2.10 May 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants