Merge pull request #92 from marcosps/templ-multifile

TemplateGen enhancements

Author: fgyanz

klpbuild/klplib/templ.py

@@ -11,7 +11,27 @@
 
 from klpbuild.klplib.bugzilla import get_bug_title
 from klpbuild.klplib.codestreams_data import get_codestreams_data
-from klpbuild.klplib.utils import ARCHS, fix_mod_string, get_mail, get_workdir, get_lp_number
+from klpbuild.klplib.utils import ARCHS, fix_mod_string, get_mail, get_workdir, get_lp_number, get_fname
+
+
+MACRO_PROTO_SYMS = """\
+<%
+def get_protos(proto_syms):
+        proto_list = []
+
+        if not proto_syms:
+            return ''
+
+        for fname, data in proto_syms.items():
+            proto_list.append(f"int {fname}_init(void);")
+            if data["cleanup"]:
+                proto_list.append(f"void {fname}_cleanup(void);\\n")
+            else:
+                proto_list.append(f"static inline void {fname}_cleanup(void);\\n")
+
+        return '\\n' + '\\n'.join(proto_list)
+%>\
+"""
 
 
 TEMPL_NO_SYMS_H = """\
@@ -38,7 +58,7 @@
 % else:
 static inline void ${ fname }_cleanup(void) {}
 % endif %
-
+${get_protos(proto_syms)}
 #else /* !IS_ENABLED(${ config }) */
 
 static inline int ${ fname }_init(void) { return 0; }
@@ -53,7 +73,7 @@
 % else:
 static inline void ${ fname }_cleanup(void) {}
 % endif
-
+${get_protos(proto_syms)}
 % endif
 #endif /* _${ fname.upper() }_H */
 """
@@ -432,107 +452,109 @@ def __generate_patched_conf(self, cs):
 
     def __generate_header_file(self, lp_path, cs):
         out_name = f"livepatch_{self.lp_name}.h"
-
-        lp_inc_dir = Path()
-        configs = set()
-        config = ""
-        mod = ""
-        has_ext = False
-        has_cleanup = False
-
-        for f, data in cs.files.items():
-            configs.add(data["conf"])
-            # If we have external symbols we need an init function to load them. If the module
-            # isn't vmlinux then we also need an _exit function
-            if data["ext_symbols"]:
-                has_ext = True
-
-                if data["module"] != "vmlinux":
-                    has_cleanup = True
-
-        # Only populate the config check in the header if the livepatch is
-        # patching code under only one config. Otherwise let the developer to
-        # fill it.
-        if len(configs) == 1:
-            config = configs.pop()
-
         render_vars = {
-            "fname": str(Path(out_name).with_suffix("")).replace("-", "_"),
-            "check_enabled": self.check_enabled,
-            "config": config,
-            "has_cleanup": has_cleanup,
+            "fname": get_fname(out_name),
         }
 
-        header_templ = TEMPL_H
-        # If we don't have any external symbols then we don't need the empty _init/_exit functions
-        if cs.needs_ibt or not has_ext:
-            header_templ = TEMPL_NO_SYMS_H
+        # We don't need any setups on IBT besides the livepatch_init/cleanup ones
+        header_templ = TEMPL_NO_SYMS_H
+
+        if not cs.needs_ibt:
+            configs = set()
+            config = ""
+            has_cleanup = False
+            proto_syms = {}
+
+            for src_file, data in cs.files.items():
+                configs.add(data["conf"])
+                # If we have external symbols we need an init function to load them. If the module
+                # isn't vmlinux then we also need an _exit function
+                if data["ext_symbols"]:
+                    if data["module"] != "vmlinux":
+                        # Used by the livepatch_cleanup
+                        has_cleanup = True
+
+                    proto_fname = get_fname(cs.lp_out_file(self.lp_name, src_file))
+                    proto_syms[proto_fname] = {"cleanup": data["module"] != "vmlinux"}
+
+            # If we don't have any external symbols then we don't need the empty _init/_exit functions
+            if proto_syms.keys():
+                # Only populate the config check in the header if the livepatch is
+                # patching code under only one config. Otherwise let the developer to
+                # fill it.
+                if len(configs) == 1:
+                    config = configs.pop()
+
+                # We there was only one entry in the proto_syms means that we have only one file in
+                # in this livepatch, so we are already covered
+                # Situations where we don't need any extra symbol prototypes:
+                # * we don't have any externalized symbols
+                # * the livepatch has only one file (_init/_cleanup for livepatch_ are created by default)
+                if len(proto_syms.keys()) == 1 and len(cs.files.keys()) == 1:
+                    proto_syms = {}
+
+                render_vars.update({
+                    "check_enabled": self.check_enabled,
+                    "config": config,
+                    "has_cleanup": has_cleanup,
+                    "proto_syms": proto_syms,
+                })
+
+                header_templ = MACRO_PROTO_SYMS + TEMPL_H
 
         with open(Path(lp_path, out_name), "w") as f:
-            lpdir = TemplateLookup(directories=[lp_inc_dir], preprocessor=TemplateGen.preproc_slashes)
+            lpdir = TemplateLookup(directories=[Path()], preprocessor=TemplateGen.preproc_slashes)
             f.write(Template(header_templ, lookup=lpdir).render(**render_vars))
 
-    def __generate_lp_file(self, lp_path, cs, src_file, use_src_name=False):
-        if src_file:
-            lp_inc_dir = str(cs.get_ccp_work_dir(self.lp_name, src_file))
-            lp_file = cs.lp_out_file(self.lp_name, src_file)
-            fdata = cs.files[str(src_file)]
-        else:
-            lp_inc_dir = Path("non-existent")
-            lp_file = None
-            fdata = {}
-
-        # if use_src_name is True, the final file will be:
-        #       bscXXXXXXX_{src_name}.c
-        # else:
-        #       livepatch_bscXXXXXXXX.c
-        if use_src_name:
-            out_name = lp_file
-        else:
-            out_name = f"livepatch_{self.lp_name}.c"
-
-        user, email = get_mail()
+    def __generate_lp_file(self, lp_path, cs, src_file, out_name):
         cve = get_codestreams_data('cve')
         if not cve:
             cve = "XXXX-XXXX"
-        cmts = get_codestreams_data('commits')
+        user, email = get_mail()
         tvars = {
-            "include_header": "livepatch_" in out_name,
+            "check_enabled": self.check_enabled,
+            "commits": get_codestreams_data('commits'),
+            "config": "CONFIG_CHANGE_ME",
             "cve": cve,
+            "email": email,
+            "fname": get_fname(out_name),
+            "include_header": "livepatch_" in out_name,
             "lp_name": self.lp_name,
             "lp_num": get_lp_number(self.lp_name),
-            "fname": str(Path(out_name).with_suffix("")).replace("-", "_"),
-            "year": datetime.today().year,
             "user": user,
-            "email": email,
-            "config": fdata.get("conf", ""),
-            "mod": fix_mod_string(fdata.get("module", "")),
-            "mod_mutex": cs.is_mod_mutex(),
-            "check_enabled": self.check_enabled,
-            "ext_vars": fdata.get("ext_symbols", ""),
-            "inc_src_file": lp_file,
-            "ibt": fdata.get("ibt", False),
-            "commits": cmts
+            "year": datetime.today().year,
         }
 
-        with open(Path(lp_path, out_name), "w") as f:
-            lpdir = TemplateLookup(directories=[lp_inc_dir], preprocessor=TemplateGen.preproc_slashes)
-
-            # If we have multiple source files for the same livepatch,
-            # create one hollow file to wire-up the multiple _init and
-            # _clean functions
-            #
-            # If we are patching a module, we should have the
-            # module_notifier armed to signal whenever the module comes on
-            # in order to do the symbol lookups. Otherwise only _init is
-            # needed, and only if there are externalized symbols being used.
-            if not lp_file:
-                temp_str = TEMPL_HOLLOW
-            elif tvars["mod"]:
+        # If we have multiple source files for the same livepatch,
+        # create one hollow file to wire-up the multiple _init and
+        # _clean functions
+        #
+        # If we are patching a module, we should have the
+        # module_notifier armed to signal whenever the module comes on
+        # in order to do the symbol lookups. Otherwise only _init is
+        # needed, and only if there are externalized symbols being used.
+        if not src_file:
+            temp_str = TEMPL_HOLLOW
+            lp_inc_dir = Path("non-existent")
+        else:
+            fdata = cs.files[str(src_file)]
+            tvars.update({
+                "config": fdata.get("conf", ""),
+                "ext_vars": fdata.get("ext_symbols", ""),
+                "ibt": fdata.get("ibt", False),
+                "inc_src_file": cs.lp_out_file(self.lp_name, src_file),
+                "mod": fix_mod_string(fdata.get("module", "")),
+                "mod_mutex": cs.is_mod_mutex(),
+            })
+
+            if tvars["mod"]:
                 temp_str = TEMPL_GET_EXTS + TEMPL_PATCH_MODULE
             else:
                 temp_str = TEMPL_GET_EXTS + TEMPL_PATCH_VMLINUX
+            lp_inc_dir = cs.get_ccp_work_dir(self.lp_name, src_file)
 
+        lpdir = TemplateLookup(directories=[lp_inc_dir], preprocessor=TemplateGen.preproc_slashes)
+        with open(Path(lp_path, out_name), "w") as f:
             f.write(Template(TEMPL_SUSE_HEADER + temp_str, lookup=lpdir).render(**tvars))
 
     def generate_livepatches(self, cs):
@@ -549,22 +571,30 @@ def generate_livepatches(self, cs):
         # config entries empty
         self.__generate_header_file(lp_path, cs)
 
-        # Run the template engine for each touched source file.
+        # Run the template engine for each generated source file.
         for src_file, _ in files.items():
-            self.__generate_lp_file(lp_path, cs, src_file, is_multi_files)
+            # if use_src_name is True, the final file will be:
+            #       bscXXXXXXX_{src_name}.c
+            # else:
+            #       livepatch_bscXXXXXXXX.c
+            out_name = f"livepatch_{self.lp_name}.c" if not is_multi_files else \
+                cs.lp_out_file(self.lp_name, src_file)
+
+            self.__generate_lp_file(lp_path, cs, src_file, out_name)
 
         # One additional file to encapsulate the _init and _clenaup methods
         # of the other source files
         if is_multi_files:
-            self.__generate_lp_file(lp_path, cs, None, False)
+            self.__generate_lp_file(lp_path, cs, None, f"livepatch_{self.lp_name}.c")
+
+        create_kbuild(self.lp_name, cs)
 
-        self.__create_kbuild(cs)
 
+def create_kbuild(lp_name, cs):
     # Create Kbuild.inc file adding an entry for all generated livepatch files.
-    def __create_kbuild(self, cs):
-        render_vars = {"bsc": self.lp_name, "cs": cs, "lpdir": cs.get_lp_dir(self.lp_name)}
-        with open(Path(cs.get_lp_dir(self.lp_name), "Kbuild.inc"), "w") as f:
-            f.write(Template(TEMPL_KBUILD).render(**render_vars))
+    render_vars = {"bsc": lp_name, "cs": cs, "lpdir": cs.get_lp_dir(lp_name)}
+    with open(Path(cs.get_lp_dir(lp_name), "Kbuild.inc"), "w") as f:
+        f.write(Template(TEMPL_KBUILD).render(**render_vars))
 
 
 def generate_commit_msg_file(lp_name):

klpbuild/klplib/utils.py

@@ -4,20 +4,20 @@
 # Author: Marcos Paulo de Souza <[email protected]>
 
 import copy
-import git
 import gzip
 import io
 import logging
 import lzma
 import os
 import platform
 import re
+import git
 import zstandard
 
 from elftools.common.utils import bytes2str
 from elftools.elf.elffile import ELFFile
 from elftools.elf.sections import SymbolTableSection
-from pathlib import PurePath
+from pathlib import Path, PurePath
 
 from natsort import natsorted
 
@@ -261,6 +261,17 @@ def fix_mod_string(mod):
     return mod.replace("-", "_")
 
 
+def get_fname(src_name):
+    """
+    Get the source name and transforms into a string version of it, without the extension.
+
+    Returns:
+        String: The same src_name string without externsion and hyphens replaced by underscores.
+    """
+
+    return str(Path(src_name).with_suffix("")).replace("-", "_")
+
+
 def get_kgraft_branch(cs_name):
     if "6.0" in cs_name:
         branch = "MICRO-6-0"

tests/test_templ.py

@@ -119,6 +119,15 @@ def test_check_header_file_included():
     assert "Upstream commit:" not in get_file_content(lp, cs, f"{lp}_kernel_events_core.c")
     assert "Upstream commit:" not in get_file_content(lp, cs, f"{lp}_net_ipv6_rpl.c")
 
+    # Check that for file kernel/events/core.c there are externalized symbols, so the prototype
+    # of init/cleanup are created on header
+    # As net/ipv6/rpl.c there are no externalized symbols we expect that it's prototype isn't
+    # created on livepatch_header file
+    header = get_file_content(lp, cs, f"livepatch_{lp}.h")
+    assert "kernel_events_core_init(void);" in header
+    assert "kernel_events_core_cleanup(void);" in header
+    assert "net_ipv6_rpl" not in header
+
 
 def test_templ_cve_specified():
     lp = "bsc_" + inspect.currentframe().f_code.co_name