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

Implement tox_loop #335

Merged
merged 2 commits into from
Apr 12, 2017
Merged

Implement tox_loop #335

merged 2 commits into from
Apr 12, 2017

Conversation

Ansa89
Copy link

@Ansa89 Ansa89 commented Dec 16, 2016

As requested by @iphydf in #131 (comment)

Supersedes #131

CC: @JFreegman @iphydf @cleverca22


This change is Reviewable

@Ansa89 Ansa89 force-pushed the c-toxcore_tox-loop branch 11 times, most recently from c88b7ad to a279d20 Compare December 16, 2016 14:14
@JFreegman
Copy link
Member

Reviewed 8 of 9 files at r2.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.


toxcore/Messenger.h, line 276 at r2 (raw file):

    bool loop_run;
    void (*loop_begin_cb)(struct Messenger *tox, void *user_data);

Change these to Messenger *m for consistency.


toxcore/tox.c, line 477 at r2 (raw file):

 * Gathers a list of every network FD activity is expected on
 * @param sockets an array of size tox_fd_count().
 */
  1. No reason to abbreviate FD.
  2. What is the max_sockets param?
  3. What does this function return?

toxcore/tox.c, line 489 at r2 (raw file):

        sock_t *tmp_sockets = (sock_t *) realloc(*sockets, fdcount * sizeof(sock_t));

        if (tmp_sockets != NULL) {

I'm not sure if I like the idea of realloc() failing silently. It's fine for now, but it's very easy to mess up.


toxcore/tox.c, line 503 at r2 (raw file):

    TCP_Connections *conns = m->net_crypto->tcp_c;

    for (i = 0; i < conns->tcp_connections_length; i++) {

Just a style suggestion: Put the 'if max_sockets == 0' clause in the loop condition.


Comments from Reviewable

@iphydf iphydf added this to the v0.1.3 milestone Dec 17, 2016
@Ansa89
Copy link
Author

Ansa89 commented Dec 17, 2016

Review status: 5 of 8 files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.


toxcore/Messenger.h, line 276 at r2 (raw file):

Previously, JFreegman wrote…

Change these to Messenger *m for consistency.

Done.


toxcore/tox.c, line 477 at r2 (raw file):

Previously, JFreegman wrote…
  1. No reason to abbreviate FD.
  2. What is the max_sockets param?
  3. What does this function return?

Done.


toxcore/tox.c, line 489 at r2 (raw file):

Previously, JFreegman wrote…

I'm not sure if I like the idea of realloc() failing silently. It's fine for now, but it's very easy to mess up.

Done.


toxcore/tox.c, line 503 at r2 (raw file):

Previously, JFreegman wrote…

Just a style suggestion: Put the 'if max_sockets == 0' clause in the loop condition.

Done.


Comments from Reviewable

@Ansa89 Ansa89 force-pushed the c-toxcore_tox-loop branch 6 times, most recently from 7da3044 to 7ea77ee Compare December 19, 2016 10:06
@iphydf iphydf modified the milestones: v0.1.3, v0.1.4 Dec 28, 2016
@Ansa89 Ansa89 force-pushed the c-toxcore_tox-loop branch 2 times, most recently from 2da685c to 5f57f4e Compare December 30, 2016 15:31
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Apr 4, 2022
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Apr 7, 2022
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Apr 10, 2022
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Apr 14, 2022
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Nov 10, 2023
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Nov 17, 2023
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Dec 9, 2023
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Dec 19, 2023
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Dec 19, 2023
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Dec 20, 2023
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Dec 27, 2023
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Dec 28, 2023
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Jan 15, 2024
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Jan 17, 2024
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Jan 24, 2024
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Jan 25, 2024
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Jan 25, 2024
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Jan 27, 2024
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Jan 31, 2024
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Jan 31, 2024
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Feb 1, 2024
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Feb 2, 2024
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Feb 2, 2024
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Feb 9, 2024
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Feb 12, 2024
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Feb 28, 2024
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Mar 5, 2024
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Mar 7, 2024
iphydf added a commit to iphydf/c-toxcore that referenced this pull request Mar 15, 2024
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature for the user, not a new feature for build script
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants