lkml.org 
[lkml]   [2022]   [Nov]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v7 2/5] x86: Add cmdline_prepare() helper
On 2022-11-10 01:54, Borislav Petkov wrote:
> On Tue, Oct 04, 2022 at 03:48:21PM +0300, Evgeniy Baskov wrote:
>> +static inline void cmdline_prepare(char *command_line,
>> + const char *boot_command_line)
>> +{
>> + if (!IS_ENABLED(CONFIG_CMDLINE_BOOL)) {
>> + strscpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
>> + } else if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
>> + /* Append boot loader cmdline to builtin one. */
>> + strlcat(command_line, " ", COMMAND_LINE_SIZE);
>> + strlcat(command_line, boot_command_line, COMMAND_LINE_SIZE);
>> + }
>
> Why can't we make this plain and simple and understandable at a quick
> glance instead of putting parts of the string in some variable and then
> still doing copying outside of the function?
>
> IOW, simply put everything in a single function:
>
> static inline void cmdline_prepare(char *dst,
> const char *builtin_cmdline,
> const char *boot_command_line)
> {
> /* Depends on CONFIG_CMDLINE_BOOL, overwrite with builtin
> cmdline */
> if (IS_ENABLED(CONFIG_CMDLINE_OVERRIDE))
> strscpy(dst, builtin_cmdline, COMMAND_LINE_SIZE);
> else if (IS_ENABLED(CONFIG_CMDLINE_BOOL)) {
> if (builtin_cmdline[0]) {
> /* append boot loader cmdline to builtin */
> srtlcat(dst, builtin_cmdline,
> COMMAND_LINE_SIZE)
> strlcat(dst, " ", COMMAND_LINE_SIZE);
> strlcat(dst, boot_command_line,
> COMMAND_LINE_SIZE);
> } else {
> strscpy(dst, boot_command_line,
> COMMAND_LINE_SIZE);
> }
> }
> }
>
> which is then called like this:
>
> cmdline_prepare(command_line, builtin_cmdline, boot_command_line);
>
> and when it returns command_line is ready. And before it, command_line
> is the empty string:
>
> static char __initdata command_line[COMMAND_LINE_SIZE];
>
> And all the complexity is in one single function and that single
> function does each case in an obvious manner and you don't have to go
> look outside of the function body to understand what it does.
>
> Hmmm?

Well, my version takes one less copy... But clarity is more important
in the kernel, I guess. I'll send updated version soon.

Thanks,
Evgeniy Baskov

\
 
 \ /
  Last update: 2022-11-10 10:00    [W:0.056 / U:0.476 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site