| Message ID | 20181105090653.7409-14-takahiro.akashi@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | efi: make efi and bootmgr more usable | expand |
On 05.11.18 10:06, AKASHI Takahiro wrote: > Those function will be used for integration with 'env' command > so as to handle uefi variables. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > cmd/efishell.c | 4 ++-- > include/command.h | 2 ++ > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/cmd/efishell.c b/cmd/efishell.c > index bd2b99e74079..8122b842dd76 100644 > --- a/cmd/efishell.c > +++ b/cmd/efishell.c > @@ -68,7 +68,7 @@ static void dump_var_data(char *data, unsigned long len) > * > * efi_$guid_$varname = {attributes}(type)value > */ > -static int do_efi_dump_var(int argc, char * const argv[]) > +int do_efi_dump_var(int argc, char * const argv[]) > { > char regex[256]; > char * const regexlist[] = {regex}; > @@ -228,7 +228,7 @@ out: > return 0; > } > > -static int do_efi_set_var(int argc, char * const argv[]) > +int do_efi_set_var(int argc, char * const argv[]) > { > char *var_name, *value = NULL; > unsigned long size = 0; > diff --git a/include/command.h b/include/command.h > index 5bb675122cce..2ce8b53f74c8 100644 > --- a/include/command.h > +++ b/include/command.h > @@ -50,6 +50,8 @@ extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); > #endif > #if defined(CONFIG_CMD_BOOTEFI) > int do_bootefi_bootmgr_exec(int boot_id); > +int do_efi_dump_var(int argc, char * const argv[]); > +int do_efi_set_var(int argc, char * const argv[]); I guess you're seeing a pattern to my comments by now. This needs to go into efi_loader.h. The respective functions need to go into lib/efi_loader. Alex
On Mon, Dec 03, 2018 at 12:54:44AM +0100, Alexander Graf wrote: > > > On 05.11.18 10:06, AKASHI Takahiro wrote: > > Those function will be used for integration with 'env' command > > so as to handle uefi variables. > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > cmd/efishell.c | 4 ++-- > > include/command.h | 2 ++ > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/cmd/efishell.c b/cmd/efishell.c > > index bd2b99e74079..8122b842dd76 100644 > > --- a/cmd/efishell.c > > +++ b/cmd/efishell.c > > @@ -68,7 +68,7 @@ static void dump_var_data(char *data, unsigned long len) > > * > > * efi_$guid_$varname = {attributes}(type)value > > */ > > -static int do_efi_dump_var(int argc, char * const argv[]) > > +int do_efi_dump_var(int argc, char * const argv[]) > > { > > char regex[256]; > > char * const regexlist[] = {regex}; > > @@ -228,7 +228,7 @@ out: > > return 0; > > } > > > > -static int do_efi_set_var(int argc, char * const argv[]) > > +int do_efi_set_var(int argc, char * const argv[]) > > { > > char *var_name, *value = NULL; > > unsigned long size = 0; > > diff --git a/include/command.h b/include/command.h > > index 5bb675122cce..2ce8b53f74c8 100644 > > --- a/include/command.h > > +++ b/include/command.h > > @@ -50,6 +50,8 @@ extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); > > #endif > > #if defined(CONFIG_CMD_BOOTEFI) > > int do_bootefi_bootmgr_exec(int boot_id); > > +int do_efi_dump_var(int argc, char * const argv[]); > > +int do_efi_set_var(int argc, char * const argv[]); > > I guess you're seeing a pattern to my comments by now. Definitely, but > This needs to go > into efi_loader.h. The respective functions need to go into lib/efi_loader. Those two functions are dedicated for command interfaces, and in my opinion, it is hard for them to be seen as part of efi_loader functionality. Thanks, -Takahiro Akashi > > > Alex
On 03.12.18 09:08, AKASHI Takahiro wrote: > On Mon, Dec 03, 2018 at 12:54:44AM +0100, Alexander Graf wrote: >> >> >> On 05.11.18 10:06, AKASHI Takahiro wrote: >>> Those function will be used for integration with 'env' command >>> so as to handle uefi variables. >>> >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >>> --- >>> cmd/efishell.c | 4 ++-- >>> include/command.h | 2 ++ >>> 2 files changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/cmd/efishell.c b/cmd/efishell.c >>> index bd2b99e74079..8122b842dd76 100644 >>> --- a/cmd/efishell.c >>> +++ b/cmd/efishell.c >>> @@ -68,7 +68,7 @@ static void dump_var_data(char *data, unsigned long len) >>> * >>> * efi_$guid_$varname = {attributes}(type)value >>> */ >>> -static int do_efi_dump_var(int argc, char * const argv[]) >>> +int do_efi_dump_var(int argc, char * const argv[]) >>> { >>> char regex[256]; >>> char * const regexlist[] = {regex}; >>> @@ -228,7 +228,7 @@ out: >>> return 0; >>> } >>> >>> -static int do_efi_set_var(int argc, char * const argv[]) >>> +int do_efi_set_var(int argc, char * const argv[]) >>> { >>> char *var_name, *value = NULL; >>> unsigned long size = 0; >>> diff --git a/include/command.h b/include/command.h >>> index 5bb675122cce..2ce8b53f74c8 100644 >>> --- a/include/command.h >>> +++ b/include/command.h >>> @@ -50,6 +50,8 @@ extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); >>> #endif >>> #if defined(CONFIG_CMD_BOOTEFI) >>> int do_bootefi_bootmgr_exec(int boot_id); >>> +int do_efi_dump_var(int argc, char * const argv[]); >>> +int do_efi_set_var(int argc, char * const argv[]); >> >> I guess you're seeing a pattern to my comments by now. > > Definitely, but > >> This needs to go >> into efi_loader.h. The respective functions need to go into lib/efi_loader. > > Those two functions are dedicated for command interfaces, and in my opinion, > it is hard for them to be seen as part of efi_loader functionality. Same comment here then :). Feel free to introduce a new header file for bootefi bits, but I'm not a big fan of a giant command.h that just exposes all possible command handlers in the world. Alex
On Sun, Dec 23, 2018 at 04:13:57AM +0100, Alexander Graf wrote: > > > On 03.12.18 09:08, AKASHI Takahiro wrote: > > On Mon, Dec 03, 2018 at 12:54:44AM +0100, Alexander Graf wrote: > >> > >> > >> On 05.11.18 10:06, AKASHI Takahiro wrote: > >>> Those function will be used for integration with 'env' command > >>> so as to handle uefi variables. > >>> > >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > >>> --- > >>> cmd/efishell.c | 4 ++-- > >>> include/command.h | 2 ++ > >>> 2 files changed, 4 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/cmd/efishell.c b/cmd/efishell.c > >>> index bd2b99e74079..8122b842dd76 100644 > >>> --- a/cmd/efishell.c > >>> +++ b/cmd/efishell.c > >>> @@ -68,7 +68,7 @@ static void dump_var_data(char *data, unsigned long len) > >>> * > >>> * efi_$guid_$varname = {attributes}(type)value > >>> */ > >>> -static int do_efi_dump_var(int argc, char * const argv[]) > >>> +int do_efi_dump_var(int argc, char * const argv[]) > >>> { > >>> char regex[256]; > >>> char * const regexlist[] = {regex}; > >>> @@ -228,7 +228,7 @@ out: > >>> return 0; > >>> } > >>> > >>> -static int do_efi_set_var(int argc, char * const argv[]) > >>> +int do_efi_set_var(int argc, char * const argv[]) > >>> { > >>> char *var_name, *value = NULL; > >>> unsigned long size = 0; > >>> diff --git a/include/command.h b/include/command.h > >>> index 5bb675122cce..2ce8b53f74c8 100644 > >>> --- a/include/command.h > >>> +++ b/include/command.h > >>> @@ -50,6 +50,8 @@ extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); > >>> #endif > >>> #if defined(CONFIG_CMD_BOOTEFI) > >>> int do_bootefi_bootmgr_exec(int boot_id); > >>> +int do_efi_dump_var(int argc, char * const argv[]); > >>> +int do_efi_set_var(int argc, char * const argv[]); > >> > >> I guess you're seeing a pattern to my comments by now. > > > > Definitely, but > > > >> This needs to go > >> into efi_loader.h. The respective functions need to go into lib/efi_loader. > > > > Those two functions are dedicated for command interfaces, and in my opinion, > > it is hard for them to be seen as part of efi_loader functionality. > > Same comment here then :). Feel free to introduce a new header file for > bootefi bits, but I'm not a big fan of a giant command.h that just > exposes all possible command handlers in the world. I've assumed that all the do_xxx() stuff should go into command.h. I will leave them in command.h for now. -Takahiro Akashi > > Alex
diff --git a/cmd/efishell.c b/cmd/efishell.c index bd2b99e74079..8122b842dd76 100644 --- a/cmd/efishell.c +++ b/cmd/efishell.c @@ -68,7 +68,7 @@ static void dump_var_data(char *data, unsigned long len) * * efi_$guid_$varname = {attributes}(type)value */ -static int do_efi_dump_var(int argc, char * const argv[]) +int do_efi_dump_var(int argc, char * const argv[]) { char regex[256]; char * const regexlist[] = {regex}; @@ -228,7 +228,7 @@ out: return 0; } -static int do_efi_set_var(int argc, char * const argv[]) +int do_efi_set_var(int argc, char * const argv[]) { char *var_name, *value = NULL; unsigned long size = 0; diff --git a/include/command.h b/include/command.h index 5bb675122cce..2ce8b53f74c8 100644 --- a/include/command.h +++ b/include/command.h @@ -50,6 +50,8 @@ extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); #endif #if defined(CONFIG_CMD_BOOTEFI) int do_bootefi_bootmgr_exec(int boot_id); +int do_efi_dump_var(int argc, char * const argv[]); +int do_efi_set_var(int argc, char * const argv[]); #endif /* common/command.c */
Those function will be used for integration with 'env' command so as to handle uefi variables. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- cmd/efishell.c | 4 ++-- include/command.h | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-)