Conversation
davidge20
requested review from
JustinCappos,
Yaxuan-w,
namanlalitnyu,
qianxichen233,
rennergade,
rupeshkoushik07,
ve1nard and
yashaswi2000
JustinCappos
left a comment
JustinCappos
left a comment
There was a problem hiding this comment.
Overall, it's a good first pass. At a high level:
-
focus on why the code does something instead of what. For example, why did you insert an item into a data structure?
-
Feel free to say when you are not sure. This is totally normal and something I do when writing my own code. Don't be shy to do this more often.
-
if you want to cite a source go ahead. For example, if you read a manpage to understand something or looked at an article, it's okay to drop a link, especially if the concept is complicated. This might be helpful for some of the setsockopt activities and related flags.
|
Please ping me for a review after you've had a chance to address all the comments. |
rennergade
left a comment
rennergade
left a comment
There was a problem hiding this comment.
Requested some minor changes. Also should add some tests, bind is used in most of the net tests in the test suite already, but worth adding some explicit edge cases.
|
@JustinCappos pinging for further review after updates |
JustinCappos
left a comment
JustinCappos
left a comment
There was a problem hiding this comment.
It's close. Please fix the things I mentioned (if something isn't clear, let me know) and then have Yash or Nick merge...
rennergade
left a comment
rennergade
left a comment
There was a problem hiding this comment.
I think this is good to go. Approved!
3 participants
Description
Commented out bind_syscall and all functions related to it