-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
Add an option to display the output of a script. (Originally battery percentage). #65
base: master
Are you sure you want to change the base?
Conversation
…cript_path'. Will soon change all instances of battery to script.
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.
great PR, hope it gets merged soon, but saw some funny stuff in the update_script procedure. also you might not want the .idea and .vscode folders, and might want to change the source branch for this PR to one without the changed readme project title, if you want to officially maintain your fork of a fork of a fork (lol the sway ecosystem experience)
FILE *pipe; | ||
char path[1035]; | ||
char output[MAX_OUTPUT_SIZE]; | ||
char *script_str = (char *)malloc((MAX_OUTPUT_SIZE + 1) * sizeof(char)); |
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.
man snprintf
- The functions snprintf()
and vsnprintf()
write at most size
bytes (including the terminating null byte ('\0'
)) to str
.
|
||
while (fgets(path, sizeof(path), pipe) != NULL) { | ||
path[strcspn(path, "\r\n")] = '\0'; | ||
strcat(output, path); |
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.
no guarantee that sizeof(output) >= strlen(output) + strlen(path) + 1
|
||
memset(output, 0, sizeof(output)); | ||
|
||
while (fgets(path, sizeof(path), pipe) != NULL) { |
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.
whole loop might not be needed as sizeof(path) > sizeof(output)
- what are we trying to do here?
pipe = popen(state.args.script_path, "r"); | ||
|
||
if (pipe == NULL) { | ||
fprintf(stderr, "Failed to run script.\n"); |
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.
this project has a swaylock_log_errno
defined somewhere, and popen
sets errno
- does the file not exist? are its permissions insufficient? etc
|
||
snprintf(script_str, MAX_OUTPUT_SIZE, "%s", output); | ||
|
||
state.args.script_str = script_str; |
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.
is a previous script_str
freed?
char path[1035]; | ||
char output[MAX_OUTPUT_SIZE]; | ||
char *script_str = (char *)malloc((MAX_OUTPUT_SIZE + 1) * sizeof(char)); |
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.
at the end of the day we're only showing a single line (I'm assuming the intent is to read only one line of the script's output) of MAX_OUTPUT_SIZE
bytes to the user, so why not just fgets
that last line into script_str? do we need to be doing all that?
This PR adds an option "--script 'path' " to display the output of a script. Also added some clarifications on moving the pam file when building from source.
If --clock is enabled, then the script output will show up on another line after the time, and if --datestr is not empty, then the script output will show up on a third line.