Address comments

Author: Kai Luo

src/symbolize/gimli.rs

@@ -278,8 +278,8 @@ struct Cache {
 struct Library {
     name: OsString,
     #[cfg(target_os = "aix")]
-    /// On AIX, the library mmapped can be a member of an big-archive file.
-    /// For example, an big-archive named libfoo.a contains libbar.so,
+    /// On AIX, the library mmapped can be a member of a big-archive file.
+    /// For example, with a big-archive named libfoo.a containing libbar.so,
     /// one can use `dlopen("libfoo.a(libbar.so)", RTLD_MEMBER | RTLD_LAZY)`
     /// to use the `libbar.so` library. In this case, only `libbar.so` is
     /// mmapped, not the whole `libfoo.a`.

src/symbolize/gimli/libs_aix.rs

@@ -26,13 +26,19 @@ pub(super) fn native_libraries() -> Vec<Library> {
             {
                 break;
             } else {
-                if Error::last_os_error().raw_os_error().unwrap() != libc::ENOMEM {
-                    return Vec::new();
+                match Error::last_os_error().raw_os_error() {
+                    Some(libc::ENOMEM) => {
+                        buffer.resize(buffer.len() * 2, mem::zeroed::<libc::ld_info>());
+                    }
+                    Some(_) => {
+                        // If other error occurs, return empty libraries.
+                        return Vec::new();
+                    }
+                    _ => unreachable!(),
                 }
-                buffer.resize(buffer.len() * 2, mem::zeroed::<libc::ld_info>());
             }
         }
-        let mut current = buffer.as_ptr() as *mut libc::ld_info;
+        let mut current = buffer.as_mut_ptr();
         loop {
             let text_base = (*current).ldinfo_textorg as usize;
             let filename_ptr: *const libc::c_char = &(*current).ldinfo_filename[0];

src/symbolize/gimli/xcoff.rs

@@ -24,15 +24,14 @@ impl Mapping {
                 Context::new(stash, Object::parse(data)?, None, None)
             } else {
                 let archive = ArchiveFile::parse(data).ok()?;
-                for member in archive.members() {
-                    if let Ok(member) = member {
-                        let name = OsStr::from_bytes(member.name());
-                        if name == member_name {
-                            let member_data = member.data(data).ok()?;
-                            if let Some(obj) = Object::parse(member_data) {
-                                return Context::new(stash, obj, None, None);
-                            }
-                        }
+                for member in archive
+                    .members()
+                    .filter_map(|m| m.ok())
+                    .filter(|m| OsStr::from_bytes(m.name()) == member_name)
+                {
+                    let member_data = member.data(data).ok()?;
+                    if let Some(obj) = Object::parse(member_data) {
+                        return Context::new(stash, obj, None, None);
                     }
                 }
                 None
@@ -64,16 +63,20 @@ pub fn parse_xcoff(data: &[u8]) -> Option<Image> {
     let _ = header.aux_header(data, &mut offset).ok()?;
     let sections = header.sections(data, &mut offset).ok()?;
     if let Some(section) = sections.iter().find(|s| {
-        let name = str::from_utf8(&s.s_name()[0..5]).unwrap();
-        return name == ".text";
+        if let Ok(name) = str::from_utf8(&s.s_name()[0..5]) {
+            name == ".text"
+        } else {
+            false
+        }
     }) {
-        return Some(Image {
+        Some(Image {
             offset: section.s_scnptr() as usize,
             base: section.s_paddr() as u64,
             size: section.s_size() as usize,
-        });
+        })
+    } else {
+        None
     }
-    return None;
 }
 
 pub fn parse_image(path: &Path, member_name: &OsString) -> Option<Image> {
@@ -83,18 +86,17 @@ pub fn parse_image(path: &Path, member_name: &OsString) -> Option<Image> {
         return parse_xcoff(data);
     } else {
         let archive = ArchiveFile::parse(data).ok()?;
-        for member in archive.members() {
-            if let Ok(member) = member {
-                let name = OsStr::from_bytes(member.name());
-                if name == member_name {
-                    let member_data = member.data(data).ok()?;
-                    if let Some(image) = parse_xcoff(member_data) {
-                        return Some(image);
-                    }
-                }
+        for member in archive
+            .members()
+            .filter_map(|m| m.ok())
+            .filter(|m| OsStr::from_bytes(m.name()) == member_name)
+        {
+            let member_data = member.data(data).ok()?;
+            if let Some(image) = parse_xcoff(member_data) {
+                return Some(image);
             }
         }
-        return None;
+        None
     }
 }
 
@@ -108,7 +110,7 @@ impl<'a> Object<'a> {
                 containing_csect: Some(index),
             } => {
                 if let Ok(tgt_sym) = file.symbol_by_index(index) {
-                    return Self::get_concrete_size(file, &tgt_sym);
+                    Self::get_concrete_size(file, &tgt_sym)
                 } else {
                     0
                 }
@@ -141,7 +143,7 @@ impl<'a> Object<'a> {
             })
             .collect::<Vec<_>>();
         syms.sort_by_key(|s| s.address);
-        return Some(Object { syms, file });
+        Some(Object { syms, file })
     }
 
     pub fn section(&self, _: &Stash, name: &str) -> Option<&'a [u8]> {

tests/smoke.rs

@@ -11,7 +11,7 @@ fn get_actual_fn_pointer(fp: usize) -> usize {
     // entry point in text section.
     //
     // For TOC, one can find more information in
-    // https://www.ibm.com/docs/en/aix/7.2?topic=program-understanding-programming-toc.
+    // https://www.ibm.com/docs/en/aix/7.2?topic=program-understanding-programming-toc
     if cfg!(target_os = "aix") {
         unsafe {
             let actual_fn_entry = *(fp as *const usize);