| Message ID | 163177341667.682366.1520674275752512771.stgit@devnote2 (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | bootconfig: Fixes to bootconfig memory management etc. | expand |
On Thu, 16 Sep 2021 15:23:36 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > Avoid using this noisy name and use more calm one. > This is just a name change. No functional change. > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > --- > include/linux/bootconfig.h | 2 +- > init/main.c | 2 +- > lib/bootconfig.c | 8 ++++---- > tools/bootconfig/main.c | 2 +- > 4 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h > index f955bb7eabbb..ba40194a339c 100644 > --- a/include/linux/bootconfig.h > +++ b/include/linux/bootconfig.h > @@ -277,7 +277,7 @@ int __init xbc_init(const char *buf, size_t size, const char **emsg, int *epos); > int __init xbc_get_info(int *node_size, size_t *data_size); > > /* XBC cleanup data structures */ > -void __init xbc_destroy_all(void); > +void __init xbc_fini(void); > > /* Debug dump functions */ > void __init xbc_debug_dump(void); > diff --git a/init/main.c b/init/main.c > index 747b4fd38a1a..99a23324d4a1 100644 > --- a/init/main.c > +++ b/init/main.c > @@ -463,7 +463,7 @@ static void __init setup_boot_config(void) > > static void __init exit_boot_config(void) > { > - xbc_destroy_all(); > + xbc_fini(); I didn't know this was a thing. But looking for other use cases with "*_fini(", there seems to be plenty of precedence in the kernel for this change. -- Steve > }
On Thu, Sep 16, 2021 at 6:26 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > > I didn't know this was a thing. But looking for other use cases with > "*_fini(", there seems to be plenty of precedence in the kernel for this > change. I wouldn't encourage it. It's an odd compiler thing, where initializers and destructors are in 'init' and 'fini' segments respectively. It makes absolutely no sense in any other context, and the fact that it has bled into kernel usage is not a good thing imnsho. Honestly, "exit" is the normal prefix/postfix, and is actually a real word. As is "destroy", used elsewhere. So I'm not going to NAK 'fini', but it's a completely stupid and pointless thing to use and there are better character sequences that aren't any more typing and are real words. Linus
On Thu, 16 Sep 2021 13:16:59 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote: > So I'm not going to NAK 'fini', but it's a completely stupid and > pointless thing to use and there are better character sequences that > aren't any more typing and are real words. I didn't like it when I first saw it, but only was going to take it because it's used elsewhere in the kernel. Because of your response, and my initial feeling about the change, I'm going to leave this patch out, and just review and accept the first three patches in the series. Masami, are you OK with that? -- Steve
On Thu, 16 Sep 2021 16:48:05 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Thu, 16 Sep 2021 13:16:59 -0700 > Linus Torvalds <torvalds@linux-foundation.org> wrote: > > > So I'm not going to NAK 'fini', but it's a completely stupid and > > pointless thing to use and there are better character sequences that > > aren't any more typing and are real words. > > I didn't like it when I first saw it, but only was going to take it because > it's used elsewhere in the kernel. > > Because of your response, and my initial feeling about the change, I'm going > to leave this patch out, and just review and accept the first three patches > in the series. > > Masami, are you OK with that? Yes, I'm OK. And I will update it to use "xbc_exit()" then. Anyway, it is good to know your opinion about this. :-) I also noticed this "_fini" recently when reviewing patches. Thank you, > > -- Steve
diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h index f955bb7eabbb..ba40194a339c 100644 --- a/include/linux/bootconfig.h +++ b/include/linux/bootconfig.h @@ -277,7 +277,7 @@ int __init xbc_init(const char *buf, size_t size, const char **emsg, int *epos); int __init xbc_get_info(int *node_size, size_t *data_size); /* XBC cleanup data structures */ -void __init xbc_destroy_all(void); +void __init xbc_fini(void); /* Debug dump functions */ void __init xbc_debug_dump(void); diff --git a/init/main.c b/init/main.c index 747b4fd38a1a..99a23324d4a1 100644 --- a/init/main.c +++ b/init/main.c @@ -463,7 +463,7 @@ static void __init setup_boot_config(void) static void __init exit_boot_config(void) { - xbc_destroy_all(); + xbc_fini(); } #else /* !CONFIG_BOOT_CONFIG */ diff --git a/lib/bootconfig.c b/lib/bootconfig.c index b088fe5c0001..43a402b02748 100644 --- a/lib/bootconfig.c +++ b/lib/bootconfig.c @@ -802,13 +802,13 @@ static int __init xbc_verify_tree(void) } /** - * xbc_destroy_all() - Clean up all parsed bootconfig + * xbc_fini() - Clean up all parsed bootconfig * * This clears all data structures of parsed bootconfig on memory. * If you need to reuse xbc_init() with new boot config, you can * use this. */ -void __init xbc_destroy_all(void) +void __init xbc_fini(void) { memblock_free_ptr(xbc_data, xbc_data_size); xbc_data = NULL; @@ -869,7 +869,7 @@ int __init xbc_init(const char *data, size_t size, const char **emsg, int *epos) if (!xbc_nodes) { if (emsg) *emsg = "Failed to allocate bootconfig nodes"; - xbc_destroy_all(); + xbc_fini(); return -ENOMEM; } memset(xbc_nodes, 0, sizeof(struct xbc_node) * XBC_NODE_MAX); @@ -925,7 +925,7 @@ int __init xbc_init(const char *data, size_t size, const char **emsg, int *epos) *epos = xbc_err_pos; if (emsg) *emsg = xbc_err_msg; - xbc_destroy_all(); + xbc_fini(); } else ret = xbc_node_num; diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c index 4f2a8d884745..84808a1871f0 100644 --- a/tools/bootconfig/main.c +++ b/tools/bootconfig/main.c @@ -397,7 +397,7 @@ static int apply_xbc(const char *path, const char *xbc_path) printf("\tChecksum: %d\n", (unsigned int)csum); /* TODO: Check the options by schema */ - xbc_destroy_all(); + xbc_fini(); free(buf); /* Remove old boot config if exists */
Avoid using this noisy name and use more calm one. This is just a name change. No functional change. Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- include/linux/bootconfig.h | 2 +- init/main.c | 2 +- lib/bootconfig.c | 8 ++++---- tools/bootconfig/main.c | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-)