Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always kill invalid file transfers when receiving file controls. #390

Merged
merged 1 commit into from
Jan 11, 2017

Conversation

iphydf
Copy link
Member

@iphydf iphydf commented Jan 5, 2017

Previously, toxcore would send a kill control to the friend only if the
file control was valid. Determining which file transfer is used does not
depend on the specific file control. We can always kill it in that case.

Also, added some logging for file control logic, since there is no other
feedback on error (failure of the file control handler is swallowed).


This change is Reviewable

@iphydf iphydf added this to the v0.1.4 milestone Jan 5, 2017
@GrayHatter
Copy link

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


toxcore/Messenger.c, line 1616 at r1 (raw file):

}

static struct File_Transfers *get_file_transfer(uint8_t receive_send, uint8_t filenumber,

bool outgoing/incoming instead of receive_send?


toxcore/Messenger.c, line 1638 at r1 (raw file):

/* return -1 on failure, 0 on success.
 */
static int handle_filecontrol(Messenger *m, int32_t friendnumber, uint8_t receive_send, uint8_t filenumber,

receive_send needs to become a bool.


Comments from Reviewable

@iphydf
Copy link
Member Author

iphydf commented Jan 8, 2017

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.


toxcore/Messenger.c, line 1616 at r1 (raw file):

Previously, GrayHatter (Gregory Mullen) wrote…

bool outgoing/incoming instead of receive_send?

Maybe, but then we should rename it everywhere. Not in this PR.


toxcore/Messenger.c, line 1638 at r1 (raw file):

Previously, GrayHatter (Gregory Mullen) wrote…

receive_send needs to become a bool.

Not at this point though. There is nothing before this call that checks whether the value is in [0,1]


Comments from Reviewable

@GrayHatter
Copy link

I'm not happy with the receive_send name. but this :lgtm:


Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@robinlinden
Copy link
Member

:lgtm_strong:


Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

Previously, toxcore would send a kill control to the friend only if the
file control was valid. Determining which file transfer is used does not
depend on the specific file control. We can always kill it in that case.

Also, added some logging for file control logic, since there is no other
feedback on error (failure of the file control handler is swallowed).
@robinlinden
Copy link
Member

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@iphydf iphydf assigned robinlinden and GrayHatter and unassigned GrayHatter Jan 10, 2017
@iphydf iphydf modified the milestone: v0.1.4 Jan 10, 2017
@iphydf iphydf assigned piling and unassigned piling Jan 10, 2017
@GrayHatter
Copy link

:lgtm:


Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@iphydf iphydf merged commit 2ba967d into TokTok:master Jan 11, 2017
@nurupo
Copy link
Member

nurupo commented Jan 12, 2017

:lgtm_strong:


Comments from Reviewable

@iphydf iphydf deleted the file-xfer-kill-invalid branch January 21, 2018 10:33
@iphydf iphydf added refactor Refactoring production code, eg. renaming a variable, not affecting semantics and removed code cleanup labels May 4, 2020
This pull request was closed.
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants