-
Notifications
You must be signed in to change notification settings - Fork 0
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 commands for configuring systemd console-mode #5
base: main
Are you sure you want to change the base?
Conversation
For any newly landing commands/modes please also open a linked issue over in blsforme so I can keep track and ensure compat is maintained, ty :) |
src/bootman/config.c
Outdated
return _write_sysconf_file(self, "console_mode", NULL); | ||
} | ||
|
||
return _write_sysconf_file(self, "console_mode", mode); |
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.
The NULL check seems a bit pointless here as you still pass NULL. Perhaps it's better to ensure all the self
vtable functions do an assert against self
being valid per the older APIS?
src/bootman/config.c
Outdated
LOG_ERROR("Failed to parse config file, defaulting to no timeout"); | ||
return -1; | ||
} | ||
|
||
return t_val; | ||
} | ||
|
||
char *boot_manager_get_console_mode(BootManager *self) | ||
{ | ||
char *value = (char*)malloc(64); |
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'm not sure I like this use of magic buffer lengths and mallocs. Also note this isn't a zero initialised (calloc) call, so any failures will result in a garbage value floating around, i.e. not NULL
. Would it not be better to simply read a single line from a file (getline
on an fp), free the internal getline pointer due to alloca crap in some implementations, then strdup the return. The console-mode check could just directly wrap it, whereas the timeout mode would simply pull it in as an autofree()
and convert to a number (atoi
)
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'm not sure I like this use of magic buffer lengths and mallocs.
I definitely don't. I changed it as suggested.
src/cli/ops/console_mode.c
Outdated
{ | ||
char list[][5] = {"", "0", "1", "2", "auto", "max", "keep"}; | ||
|
||
for(size_t i = 0; i < sizeof list; i++) { |
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.
Not sure why there isn't a space after for
- is this why the clang-format file was changed? Looks inconsistent :D
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 this why the clang-format file was changed?
No, it was changed because my editor didn't pick it up (as it was invalid). I probably wrote this line before fixing the file 😞.
I ran a clang-format on all the changed bits, so those should be OK now.
return false; | ||
} | ||
|
||
bool cbm_command_set_console_mode(int argc, char **argv) |
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.
For the benefit of looking back at this project for blsforme work could we return to using doxygen style comments for all non static APIs?
* published by the Free Software Foundation; either version 2.1 | ||
* of the License, or (at your option) any later version. | ||
*/ | ||
|
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.
TBH the Solus fork needs to add a copyright section to impacted files since The Events, post 2018. Otherwise only Intel is benefiting from protection of their (and my old) copyrights, rather than Solus for current works.
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.
minor clean up changes requested :)
4a33b97
to
20eddb7
Compare
20eddb7
to
24e6a66
Compare
Add
set-console-mode
andget-console-mode
commands for managing theconsole-mode
in systemd.Resolves #2.