Reorder pid argument in normalization APIs

The pid argument in the normalization APIs fulfills pretty much the same job as the src in the symbolization logic. As such, we should order them consistently with respect to the addresses provided. With this change we move the argument before the address list. Signed-off-by: Daniel Müller <[email protected]>
libbpf · Dec 19, 2023 · 1d78b0b · 1d78b0b
1 parent 0f68e4c
commit 1d78b0b
Show file tree

Hide file tree

Showing 9 changed files with 24 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@ Unreleased
 - Introduced `symbolize::Reason` enum to provide best guess at
   why symbolization was not successful as part of the
   `symbolize::Symbolized::Unknown` variant
+- Reordered `pid` argument to normalization functions before addresses
 
 
 0.2.0-alpha.9

diff --git a/benches/normalize.rs b/benches/normalize.rs
@@ -22,7 +22,7 @@ fn normalize_process() {
 
     let normalizer = Normalizer::new();
     let normalized = normalizer
-        .normalize_user_addrs_sorted(black_box(addrs.as_slice()), black_box(0.into()))
+        .normalize_user_addrs_sorted(black_box(0.into()), black_box(addrs.as_slice()))
         .unwrap();
     assert_eq!(normalized.outputs.len(), 5);
 }

diff --git a/capi/include/blazesym.h b/capi/include/blazesym.h
@@ -547,9 +547,9 @@ void blaze_normalizer_free(blaze_normalizer *normalizer);
  * `addr_cnt` addresses.
  */
 struct blaze_normalized_user_output *blaze_normalize_user_addrs(const blaze_normalizer *normalizer,
+                                                                uint32_t pid,
                                                                 const uintptr_t *addrs,
-                                                                size_t addr_cnt,
-                                                                uint32_t pid);
+                                                                size_t addr_cnt);
 
 /**
  * Normalize a list of user space addresses.
@@ -571,9 +571,9 @@ struct blaze_normalized_user_output *blaze_normalize_user_addrs(const blaze_norm
  * `addr_cnt` addresses.
  */
 struct blaze_normalized_user_output *blaze_normalize_user_addrs_sorted(const blaze_normalizer *normalizer,
+                                                                       uint32_t pid,
                                                                        const uintptr_t *addrs,
-                                                                       size_t addr_cnt,
-                                                                       uint32_t pid);
+                                                                       size_t addr_cnt);
 
 /**
  * Free an object as returned by [`blaze_normalize_user_addrs`] or

diff --git a/capi/src/normalize.rs b/capi/src/normalize.rs
@@ -362,17 +362,17 @@ impl From<UserOutput> for blaze_normalized_user_output {
 #[no_mangle]
 pub unsafe extern "C" fn blaze_normalize_user_addrs(
     normalizer: *const blaze_normalizer,
+    pid: u32,
     addrs: *const Addr,
     addr_cnt: usize,
-    pid: u32,
 ) -> *mut blaze_normalized_user_output {
     // SAFETY: The caller needs to ensure that `normalizer` is a valid
     //         pointer.
     let normalizer = unsafe { &*normalizer };
     // SAFETY: The caller needs to ensure that `addrs` is a valid pointer and
     //         that it points to `addr_cnt` elements.
     let addrs = unsafe { slice_from_user_array(addrs, addr_cnt) };
-    let result = normalizer.normalize_user_addrs(addrs, pid.into());
+    let result = normalizer.normalize_user_addrs(pid.into(), addrs);
     match result {
         Ok(addrs) => Box::into_raw(Box::new(blaze_normalized_user_output::from(addrs))),
         Err(_err) => ptr::null_mut(),
@@ -400,17 +400,17 @@ pub unsafe extern "C" fn blaze_normalize_user_addrs(
 #[no_mangle]
 pub unsafe extern "C" fn blaze_normalize_user_addrs_sorted(
     normalizer: *const blaze_normalizer,
+    pid: u32,
     addrs: *const Addr,
     addr_cnt: usize,
-    pid: u32,
 ) -> *mut blaze_normalized_user_output {
     // SAFETY: The caller needs to ensure that `normalizer` is a valid
     //         pointer.
     let normalizer = unsafe { &*normalizer };
     // SAFETY: The caller needs to ensure that `addrs` is a valid pointer and
     //         that it points to `addr_cnt` elements.
     let addrs = unsafe { slice_from_user_array(addrs, addr_cnt) };
-    let result = normalizer.normalize_user_addrs_sorted(addrs, pid.into());
+    let result = normalizer.normalize_user_addrs_sorted(pid.into(), addrs);
     match result {
         Ok(addrs) => Box::into_raw(Box::new(blaze_normalized_user_output::from(addrs))),
         Err(_err) => ptr::null_mut(),
@@ -598,7 +598,7 @@ mod tests {
         assert_ne!(normalizer, ptr::null_mut());
 
         let result = unsafe {
-            blaze_normalize_user_addrs(normalizer, addrs.as_slice().as_ptr(), addrs.len(), 0)
+            blaze_normalize_user_addrs(normalizer, 0, addrs.as_slice().as_ptr(), addrs.len())
         };
         assert_ne!(result, ptr::null_mut());
 
@@ -626,7 +626,7 @@ mod tests {
         assert_ne!(normalizer, ptr::null_mut());
 
         let result = unsafe {
-            blaze_normalize_user_addrs_sorted(normalizer, addrs.as_slice().as_ptr(), addrs.len(), 0)
+            blaze_normalize_user_addrs_sorted(normalizer, 0, addrs.as_slice().as_ptr(), addrs.len())
         };
         assert_ne!(result, ptr::null_mut());
 

diff --git a/cli/src/main.rs b/cli/src/main.rs
@@ -44,7 +44,7 @@ fn normalize(normalize: args::Normalize) -> Result<()> {
     match normalize {
         args::Normalize::User(args::User { pid, addrs }) => {
             let normalized = normalizer
-                .normalize_user_addrs(addrs.as_slice(), pid)
+                .normalize_user_addrs(pid, addrs.as_slice())
                 .context("failed to normalize addresses")?;
             for (addr, (output, meta_idx)) in addrs.iter().zip(&normalized.outputs) {
                 print!("{addr:#016x}: ");

diff --git a/src/normalize/meta.rs b/src/normalize/meta.rs
@@ -22,7 +22,7 @@ type BuildId = Vec<u8>;
 /// let addr_in_elf_in_apk = capture_addr_in_elf_in_apk();
 /// let normalizer = normalize::Normalizer::new();
 /// let normalized = normalizer
-///     .normalize_user_addrs_sorted([addr_in_elf_in_apk].as_slice(), Pid::Slf)
+///     .normalize_user_addrs_sorted(Pid::Slf, [addr_in_elf_in_apk].as_slice())
 ///     .unwrap();
 /// let (output, meta_idx) = normalized.outputs[0];
 /// let meta = &normalized.meta[meta_idx];

diff --git a/src/normalize/mod.rs b/src/normalize/mod.rs
@@ -26,7 +26,7 @@
 //! let fopen_addr = libc::fopen as Addr;
 //! let addrs = [fopen_addr];
 //! let pid = Pid::Slf;
-//! let normalized = normalizer.normalize_user_addrs(&addrs, pid).unwrap();
+//! let normalized = normalizer.normalize_user_addrs(pid, &addrs).unwrap();
 //! assert_eq!(normalized.outputs.len(), 1);
 //!
 //! let (file_offset, meta_idx) = normalized.outputs[0];

diff --git a/src/normalize/normalizer.rs b/src/normalize/normalizer.rs
@@ -113,7 +113,7 @@ impl Normalizer {
     /// Normalized addresses are reported in the exact same order in which the
     /// non-normalized ones were provided.
     #[cfg_attr(feature = "tracing", crate::log::instrument(skip(self)))]
-    pub fn normalize_user_addrs_sorted(&self, addrs: &[Addr], pid: Pid) -> Result<UserOutput> {
+    pub fn normalize_user_addrs_sorted(&self, pid: Pid, addrs: &[Addr]) -> Result<UserOutput> {
         normalize_user_addrs_sorted_impl(addrs.iter().copied(), pid, self.build_ids)
     }
 
@@ -127,7 +127,7 @@ impl Normalizer {
     /// [`Normalizer::normalize_user_addrs_sorted`] instead will result in
     /// slightly faster normalization.
     #[cfg_attr(feature = "tracing", crate::log::instrument(skip(self)))]
-    pub fn normalize_user_addrs(&self, addrs: &[Addr], pid: Pid) -> Result<UserOutput> {
+    pub fn normalize_user_addrs(&self, pid: Pid, addrs: &[Addr]) -> Result<UserOutput> {
         util::with_ordered_elems(
             addrs,
             |normalized: &mut UserOutput| normalized.outputs.as_mut_slice(),
@@ -173,7 +173,7 @@ mod tests {
 
         let normalizer = Normalizer::new();
         let err = normalizer
-            .normalize_user_addrs_sorted(addrs.as_slice(), Pid::Slf)
+            .normalize_user_addrs_sorted(Pid::Slf, addrs.as_slice())
             .unwrap_err();
         assert!(err.to_string().contains("are not sorted"), "{err}");
     }
@@ -187,7 +187,7 @@ mod tests {
 
         let normalizer = Normalizer::new();
         let normalized = normalizer
-            .normalize_user_addrs_sorted(addrs.as_slice(), Pid::Slf)
+            .normalize_user_addrs_sorted(Pid::Slf, addrs.as_slice())
             .unwrap();
         assert_eq!(normalized.outputs.len(), 2);
         assert_eq!(normalized.meta.len(), 1);
@@ -216,7 +216,7 @@ mod tests {
 
         let normalizer = Normalizer::new();
         let normalized = normalizer
-            .normalize_user_addrs(addrs.as_slice(), Pid::Slf)
+            .normalize_user_addrs(Pid::Slf, addrs.as_slice())
             .unwrap();
         assert_eq!(normalized.outputs.len(), 6);
 
@@ -265,7 +265,7 @@ mod tests {
 
         let normalizer = Normalizer::new();
         let normalized = normalizer
-            .normalize_user_addrs_sorted([the_answer_addr as Addr].as_slice(), Pid::Slf)
+            .normalize_user_addrs_sorted(Pid::Slf, [the_answer_addr as Addr].as_slice())
             .unwrap();
         assert_eq!(normalized.outputs.len(), 1);
         assert_eq!(normalized.meta.len(), 1);
@@ -329,7 +329,7 @@ mod tests {
 
             let normalizer = Normalizer::new();
             let normalized = normalizer
-                .normalize_user_addrs_sorted([the_answer_addr as Addr].as_slice(), Pid::Slf)
+                .normalize_user_addrs_sorted(Pid::Slf, [the_answer_addr as Addr].as_slice())
                 .unwrap();
             assert_eq!(normalized.outputs.len(), 1);
             assert_eq!(normalized.meta.len(), 1);

diff --git a/tests/blazesym.rs b/tests/blazesym.rs
@@ -362,7 +362,7 @@ fn normalize_elf_addr() {
 
         let normalizer = Normalizer::new();
         let normalized = normalizer
-            .normalize_user_addrs_sorted([the_answer_addr as Addr].as_slice(), Pid::Slf)
+            .normalize_user_addrs_sorted(Pid::Slf, [the_answer_addr as Addr].as_slice())
             .unwrap();
         assert_eq!(normalized.outputs.len(), 1);
         assert_eq!(normalized.meta.len(), 1);
@@ -417,7 +417,7 @@ fn normalize_build_id_rading() {
             .enable_build_ids(read_build_ids)
             .build();
         let normalized = normalizer
-            .normalize_user_addrs_sorted([the_answer_addr as Addr].as_slice(), Pid::Slf)
+            .normalize_user_addrs_sorted(Pid::Slf, [the_answer_addr as Addr].as_slice())
             .unwrap();
         assert_eq!(normalized.outputs.len(), 1);
         assert_eq!(normalized.meta.len(), 1);