From f22d5681ff7b8c42037d6b96e83f2d59f27c5b9a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20M=C3=BCller?= <deso@posteo.net>
Date: Mon, 20 Nov 2023 11:59:59 -0800
Subject: [PATCH] Remove 'mutable' argument to gen_skel_map_defs()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There is not too much of a point in passing in the desired mutability to
gen_skel_map_defs() when all call sites invoke it with both true and
false.
Move the logic for dealing with mutability into the function itself to
shield callers from the need to care.

Signed-off-by: Daniel Müller <deso@posteo.net>
---
 libbpf-cargo/src/gen/mod.rs | 105 ++++++++++++++++++------------------
 1 file changed, 54 insertions(+), 51 deletions(-)

diff --git a/libbpf-cargo/src/gen/mod.rs b/libbpf-cargo/src/gen/mod.rs
index 349075aa..f6bbd52d 100644
--- a/libbpf-cargo/src/gen/mod.rs
+++ b/libbpf-cargo/src/gen/mod.rs
@@ -267,66 +267,71 @@ fn gen_skel_map_defs(
     object: &mut BpfObj,
     obj_name: &str,
     open: bool,
-    mutable: bool,
 ) -> Result<()> {
-    if MapIter::new(object.as_mut_ptr()).next().is_none() {
-        return Ok(());
-    }
-
-    let (struct_suffix, mut_prefix, map_fn) = if mutable {
-        ("Mut", "mut ", "map_mut")
-    } else {
-        ("", "", "map")
-    };
-
-    let (struct_name, inner_ty, return_ty) = if open {
-        (
-            format!("Open{obj_name}Maps{struct_suffix}"),
-            "libbpf_rs::OpenObject",
-            "libbpf_rs::OpenMap",
-        )
-    } else {
-        (
-            format!("{obj_name}Maps{struct_suffix}"),
-            "libbpf_rs::Object",
-            "libbpf_rs::Map",
-        )
-    };
-
-    write!(
-        skel,
-        r#"
-        pub struct {struct_name}<'a> {{
-            inner: &'a {mut_prefix}{inner_ty},
-        }}
+    let mut gen = |mutable| -> Result<()> {
+        if MapIter::new(object.as_mut_ptr()).next().is_none() {
+            return Ok(());
+        }
 
-        impl {struct_name}<'_> {{
-        "#,
-    )?;
+        let (struct_suffix, mut_prefix, map_fn) = if mutable {
+            ("Mut", "mut ", "map_mut")
+        } else {
+            ("", "", "map")
+        };
 
-    for map in MapIter::new(object.as_mut_ptr()) {
-        let map_name = match get_map_name(map)? {
-            Some(n) => n,
-            None => continue,
+        let (struct_name, inner_ty, return_ty) = if open {
+            (
+                format!("Open{obj_name}Maps{struct_suffix}"),
+                "libbpf_rs::OpenObject",
+                "libbpf_rs::OpenMap",
+            )
+        } else {
+            (
+                format!("{obj_name}Maps{struct_suffix}"),
+                "libbpf_rs::Object",
+                "libbpf_rs::Map",
+            )
         };
 
         write!(
             skel,
             r#"
-            pub fn {map_name}(&{mut_prefix}self) -> &{mut_prefix}{return_ty} {{
-                self.inner.{map_fn}("{raw_map_name}").unwrap()
+            pub struct {struct_name}<'a> {{
+                inner: &'a {mut_prefix}{inner_ty},
             }}
+
+            impl {struct_name}<'_> {{
             "#,
-            map_name = map_name,
-            raw_map_name = get_raw_map_name(map)?,
-            return_ty = return_ty,
-            mut_prefix = mut_prefix,
-            map_fn = map_fn
         )?;
-    }
 
-    writeln!(skel, "}}")?;
+        for map in MapIter::new(object.as_mut_ptr()) {
+            let map_name = match get_map_name(map)? {
+                Some(n) => n,
+                None => continue,
+            };
+
+            write!(
+                skel,
+                r#"
+                pub fn {map_name}(&{mut_prefix}self) -> &{mut_prefix}{return_ty} {{
+                    self.inner.{map_fn}("{raw_map_name}").unwrap()
+                }}
+                "#,
+                map_name = map_name,
+                raw_map_name = get_raw_map_name(map)?,
+                return_ty = return_ty,
+                mut_prefix = mut_prefix,
+                map_fn = map_fn
+            )?;
+        }
 
+        writeln!(skel, "}}")?;
+
+        Ok(())
+    };
+
+    let () = gen(true)?;
+    let () = gen(false)?;
     Ok(())
 }
 
@@ -745,8 +750,7 @@ fn gen_skel_contents(_debug: bool, raw_obj_name: &str, obj_file_path: &Path) ->
         name = obj_name
     )?;
 
-    gen_skel_map_defs(&mut skel, &mut object, &obj_name, true, false)?;
-    gen_skel_map_defs(&mut skel, &mut object, &obj_name, true, true)?;
+    gen_skel_map_defs(&mut skel, &mut object, &obj_name, true)?;
     gen_skel_prog_defs(&mut skel, &mut object, &obj_name, true, false)?;
     gen_skel_prog_defs(&mut skel, &mut object, &obj_name, true, true)?;
     gen_skel_datasec_defs(&mut skel, raw_obj_name, &mmap)?;
@@ -799,8 +803,7 @@ fn gen_skel_contents(_debug: bool, raw_obj_name: &str, obj_file_path: &Path) ->
     gen_skel_datasec_getters(&mut skel, &mut object, raw_obj_name, false)?;
     writeln!(skel, "}}")?;
 
-    gen_skel_map_defs(&mut skel, &mut object, &obj_name, false, false)?;
-    gen_skel_map_defs(&mut skel, &mut object, &obj_name, false, true)?;
+    gen_skel_map_defs(&mut skel, &mut object, &obj_name, false)?;
     gen_skel_prog_defs(&mut skel, &mut object, &obj_name, false, false)?;
     gen_skel_prog_defs(&mut skel, &mut object, &obj_name, false, true)?;
     gen_skel_link_defs(&mut skel, &mut object, &obj_name)?;