Avoid leaking memory when handling error cases

We need to always handle the cleanup of resources even when we are catching errors so to avoid a lot of code duplication we wrap the internal logic into a lambda. Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
OSSystems · Sep 15, 2020 · c15c3b3 · c15c3b3
1 parent 357a06f
commit c15c3b3
Showing 1 changed file with 61 additions and 57 deletions.
diff --git a/src/lib.rs b/src/lib.rs
@@ -305,79 +305,83 @@ where
         let archive_reader = ffi::archive_read_new();
         let archive_writer = ffi::archive_write_disk_new();
 
-        archive_result(
-            ffi::archive_read_support_filter_all(archive_reader),
-            archive_reader,
-        )?;
-
-        match mode {
-            Mode::RawFormat => archive_result(
-                ffi::archive_read_support_format_raw(archive_reader),
-                archive_reader,
-            )?,
-            Mode::AllFormat => archive_result(
-                ffi::archive_read_support_format_all(archive_reader),
+        let res = (|| {
+            archive_result(
+                ffi::archive_read_support_filter_all(archive_reader),
                 archive_reader,
-            )?,
-            Mode::WriteDisk { ownership } => {
-                let mut writer_flags = ffi::ARCHIVE_EXTRACT_TIME
-                    | ffi::ARCHIVE_EXTRACT_PERM
-                    | ffi::ARCHIVE_EXTRACT_ACL
-                    | ffi::ARCHIVE_EXTRACT_FFLAGS
-                    | ffi::ARCHIVE_EXTRACT_XATTR;
-
-                if let Ownership::Preserve = ownership {
-                    writer_flags |= ffi::ARCHIVE_EXTRACT_OWNER;
-                };
-
-                archive_result(
-                    ffi::archive_write_disk_set_options(archive_writer, writer_flags as i32),
-                    archive_writer,
-                )?;
-                archive_result(
-                    ffi::archive_write_disk_set_standard_lookup(archive_writer),
-                    archive_writer,
-                )?;
-                archive_result(
-                    ffi::archive_read_support_format_all(archive_reader),
-                    archive_reader,
-                )?;
-                archive_result(
+            )?;
+
+            match mode {
+                Mode::RawFormat => archive_result(
                     ffi::archive_read_support_format_raw(archive_reader),
                     archive_reader,
-                )?;
+                )?,
+                Mode::AllFormat => archive_result(
+                    ffi::archive_read_support_format_all(archive_reader),
+                    archive_reader,
+                )?,
+                Mode::WriteDisk { ownership } => {
+                    let mut writer_flags = ffi::ARCHIVE_EXTRACT_TIME
+                        | ffi::ARCHIVE_EXTRACT_PERM
+                        | ffi::ARCHIVE_EXTRACT_ACL
+                        | ffi::ARCHIVE_EXTRACT_FFLAGS
+                        | ffi::ARCHIVE_EXTRACT_XATTR;
+
+                    if let Ownership::Preserve = ownership {
+                        writer_flags |= ffi::ARCHIVE_EXTRACT_OWNER;
+                    };
+
+                    archive_result(
+                        ffi::archive_write_disk_set_options(archive_writer, writer_flags as i32),
+                        archive_writer,
+                    )?;
+                    archive_result(
+                        ffi::archive_write_disk_set_standard_lookup(archive_writer),
+                        archive_writer,
+                    )?;
+                    archive_result(
+                        ffi::archive_read_support_format_all(archive_reader),
+                        archive_reader,
+                    )?;
+                    archive_result(
+                        ffi::archive_read_support_format_raw(archive_reader),
+                        archive_reader,
+                    )?;
+                }
             }
-        }
 
-        if archive_reader.is_null() || archive_writer.is_null() {
-            return Err(Error::NullArchive);
-        }
+            if archive_reader.is_null() || archive_writer.is_null() {
+                return Err(Error::NullArchive);
+            }
 
-        let mut pipe = Pipe {
-            reader: &mut reader,
-            buffer: &mut [0; READER_BUFFER_SIZE],
-        };
+            let mut pipe = Pipe {
+                reader: &mut reader,
+                buffer: &mut [0; READER_BUFFER_SIZE],
+            };
 
-        archive_result(
-            ffi::archive_read_open(
+            archive_result(
+                ffi::archive_read_open(
+                    archive_reader,
+                    (&mut pipe as *mut Pipe) as *mut c_void,
+                    None,
+                    Some(libarchive_read_callback),
+                    None,
+                ),
                 archive_reader,
-                (&mut pipe as *mut Pipe) as *mut c_void,
-                None,
-                Some(libarchive_read_callback),
-                None,
-            ),
-            archive_reader,
-        )?;
+            )?;
 
-        let res = f(archive_reader, archive_writer, archive_entry)?;
+            f(archive_reader, archive_writer, archive_entry)
+        })();
 
         archive_result(ffi::archive_read_close(archive_reader), archive_reader)?;
         archive_result(ffi::archive_read_free(archive_reader), archive_reader)?;
+
         archive_result(ffi::archive_write_close(archive_writer), archive_writer)?;
         archive_result(ffi::archive_write_free(archive_writer), archive_writer)?;
+
         ffi::archive_entry_free(archive_entry);
 
-        Ok(res)
+        res
     }
 }