-
Notifications
You must be signed in to change notification settings - Fork 135
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
shredder: use compile-time length #709
Conversation
0c37846
to
965bf81
Compare
Looks good to me! Do you want this merged asap or should we wait for something else? (I haven't fully understood what the idea about master/develop is so I don't want to mess anything up :p) |
It's not important, this commit is a detail. However, I think more effort should be done on the GUI installation (be convenient for package maintainers, make sure that the Python package is well installed, etc) before release :( |
lib/cmdline.c
Outdated
@@ -176,7 +176,7 @@ static void rm_cmd_start_gui(int argc, const char **argv) { | |||
return; | |||
} | |||
|
|||
if(write(bootstrap_fd, RM_PY_BOOTSTRAP, strlen(RM_PY_BOOTSTRAP)) < 0) { | |||
if(write(bootstrap_fd, rm_py_bootstrap, sizeof rm_py_bootstrap - 1) < 0) { |
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.
BTW, I think sizeof(rm_py_bootstrap) - 1
would be better for readability, but in this case it's a matter of taste.
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.
I don't mind changing it, there are inconsistency in the rest of the code, but most often than not it is used with parenthesis.
I changed RM_PY_BOOTSTRAP to lower-case because usually uppercase is for preprocessor macros. But again, that's a matter of taste.
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.
Yeah, I agree on the casing.
965bf81
to
4e8dcfe
Compare
- use compile-time length - remove trailing spaces - fix comment
4e8dcfe
to
dfbff61
Compare
No description provided.