Thread (43 messages) 43 messages, 6 authors, 2022-02-28

Re: [PATCH v8 04/13] module: Move livepatch support to a separate file

From: Aaron Tomlin <hidden>
Date: 2022-02-25 10:34:09
Also in: lkml

On Fri 2022-02-25 10:34 +0100, Petr Mladek wrote:
quoted hunk ↗ jump to hunk
This code include several other well hidden changes:
--- del.p    2022-02-24 16:55:26.570054922 +0100
+++ add.p    2022-02-24 16:56:04.766781394 +0100
@@ -3,14 +3,14 @@
  * section header table, section string table, and symtab section
  * index from info to mod->klp_info.
  */
-static int copy_module_elf(struct module *mod, struct load_info *info)
+int copy_module_elf(struct module *mod, struct load_info *info)
 {
     unsigned int size, symndx;
     int ret;

     size = sizeof(*mod->klp_info);
     mod->klp_info = kmalloc(size, GFP_KERNEL);
-    if (mod->klp_info == NULL)
+    if (!mod->klp_info)
         return -ENOMEM;

     /* Elf header */
@@ -20,7 +20,7 @@ static int copy_module_elf(struct module
     /* Elf section header table */
     size = sizeof(*info->sechdrs) * info->hdr->e_shnum;
     mod->klp_info->sechdrs = kmemdup(info->sechdrs, size, GFP_KERNEL);
-    if (mod->klp_info->sechdrs == NULL) {
+    if (!mod->klp_info->sechdrs) {
         ret = -ENOMEM;
         goto free_info;
     }
@@ -28,7 +28,7 @@ static int copy_module_elf(struct module
     /* Elf section name string table */
     size = info->sechdrs[info->hdr->e_shstrndx].sh_size;
     mod->klp_info->secstrings = kmemdup(info->secstrings, size, GFP_KERNEL);
-    if (mod->klp_info->secstrings == NULL) {
+    if (!mod->klp_info->secstrings) {
         ret = -ENOMEM;
         goto free_sechdrs;
     }
@@ -43,8 +43,7 @@ static int copy_module_elf(struct module
      * to core_kallsyms.symtab since the copy of the symtab in module
      * init memory is freed at the end of do_init_module().
      */
-    mod->klp_info->sechdrs[symndx].sh_addr = \
-        (unsigned long) mod->core_kallsyms.symtab;
+    mod->klp_info->sechdrs[symndx].sh_addr = (unsigned long)mod->core_kallsyms.symtab;

     return 0;

Please do not do these small coding style changes. It complicates the
review and increases the risk of regressions. Different people
have different preferences. Just imagine that every half a year
someone update style of a code by his personal preferences. The
real changes will then get lost in a lot of noise.

Coding style changes might be acceptable only when the code is
reworked or when it significantly improves readability.


That said. I reviewed and tested this patch and did not find any
problem. Feel free to use:

Reviewed-by: Petr Mladek <pmladek@suse.com>
Tested-by: Petr Mladek <pmladek@suse.com>

Please, take the above as an advice for your future work.
Hi Petr,

Firstly, thank you for your feedback. Fair enough and noted.


Kind regards,

-- 
Aaron Tomlin
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help