-
Notifications
You must be signed in to change notification settings - Fork 282
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
Avoid multiple for-next expressions. #1041
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 2 files at r1.
Reviewable status: 0 of 1 LGTMs obtained
toxcore/DHT.c, line 1709 at r1 (raw file):
IPPTsPng *const assocs[] = { &client->assoc6, &client->assoc4 }; for (uint32_t j = 0; j < 2; ++j) {
could use sizeof(assocs)
instead of hardcoding 2 here, but it comes with it's own drawbacks, what do you think? Or move to the same style as in the other file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 1 LGTMs obtained
toxcore/DHT.c, line 1709 at r1 (raw file):
Previously, sudden6 wrote…
could use
sizeof(assocs)
instead of hardcoding 2 here, but it comes with it's own drawbacks, what do you think? Or move to the same style as in the other file?
It's sizeof(assocs) / sizeof(assocs[0])
, which is a bit wordy. This is the only case throughout toxcore where we need this form of iteration over assocs, (though a pointer subtraction could replace j
), because the value (0 or 1) is actually used. The formula below looks a bit convoluted and there may be a better way, but I don't want to figure that one out right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 1 LGTMs obtained
toxcore/DHT.c, line 1709 at r1 (raw file):
Previously, iphydf wrote…
It's
sizeof(assocs) / sizeof(assocs[0])
, which is a bit wordy. This is the only case throughout toxcore where we need this form of iteration over assocs, (though a pointer subtraction could replacej
), because the value (0 or 1) is actually used. The formula below looks a bit convoluted and there may be a better way, but I don't want to figure that one out right now.
could be extracted to a constant const size_t assocs_len = ...
which would be clear and only one additional line, I really don't like hardcoding that number here
2640447
to
41f924b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 1 LGTMs obtained
toxcore/DHT.c, line 1709 at r1 (raw file):
Previously, sudden6 wrote…
could be extracted to a constant
const size_t assocs_len = ...
which would be clear and only one additional line, I really don't like hardcoding that number here
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status:
complete! 1 of 1 LGTMs obtained
All for-loops in toxcore are of the form for (<for-init>; <for-cond>; <for-next>) { <body> } `for-init` can be a variable declaration (like `int i = 0`), an assignment (like `i = 0`), or empty. `for-cond` can be any expression. `for-next` can be an assignment or a single increment/decrement expression (like `++i` or `--i`). No other forms are allowed, so e.g. comma expressions in any of these are not allowed (so no `for (i = 0, j = n; ...; ++i, --j)`).
41f924b
to
7f8b29c
Compare
All for-loops in toxcore are of the form
for-init
can be a variable declaration (likeint i = 0
), anassignment (like
i = 0
), or empty.for-cond
can be any expression.for-next
can be an assignment or a single increment/decrementexpression (like
++i
or--i
).No other forms are allowed, so e.g. comma expressions in any of these are
not allowed (so no
for (i = 0, j = n; ...; ++i, --j)
).This change is![Reviewable](https://camo.githubusercontent.com/23b05f5fb48215c989e92cc44cf6512512d083132bd3daf689867c8d9d386888/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)