From dab75a234f12db3932bbf1285581ee788d7c7c16 Mon Sep 17 00:00:00 2001
From: Jiang Liu <gerry@linux.alibaba.com>
Date: Fri, 3 Mar 2023 09:28:32 +0800
Subject: [PATCH] nydus-image: minor improvement to nydus-image

Minor improvement to nydus-image:
- better handling of `chunk-size` argument
- avoid assert at runtime by returning error code

Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>
---
 src/bin/nydus-image/builder/tarball.rs | 23 ++++++++++++++---------
 src/bin/nydus-image/main.rs            | 11 ++++++-----
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/src/bin/nydus-image/builder/tarball.rs b/src/bin/nydus-image/builder/tarball.rs
index 2c52b652713..17153d9d502 100644
--- a/src/bin/nydus-image/builder/tarball.rs
+++ b/src/bin/nydus-image/builder/tarball.rs
@@ -149,7 +149,7 @@ impl<'a> TarballTreeBuilder<'a> {
 
         // Generate the root node in advance, it may be overwritten by entries from the tar stream.
         let mut nodes = Vec::with_capacity(10240);
-        let root = self.create_directory(Path::new("/"), &nodes)?;
+        let root = self.create_directory(Path::new("/"), 0)?;
         nodes.push(root.clone());
 
         // Generate RAFS node for each tar entry, and optionally adding missing parents.
@@ -192,10 +192,13 @@ impl<'a> TarballTreeBuilder<'a> {
     ) -> Result<Node> {
         let header = entry.header();
         let entry_type = header.entry_type();
-        assert!(!entry_type.is_gnu_longname());
-        assert!(!entry_type.is_gnu_longlink());
-        assert!(!entry_type.is_pax_local_extensions());
-        if entry_type.is_pax_global_extensions() {
+        if entry_type.is_gnu_longname() {
+            return Err(anyhow!("unsupported gnu_longname from tar header"));
+        } else if entry_type.is_gnu_longlink() {
+            return Err(anyhow!("unsupported gnu_longlink from tar header"));
+        } else if entry_type.is_pax_local_extensions() {
+            return Err(anyhow!("unsupported pax_local_extensions from tar header"));
+        } else if entry_type.is_pax_global_extensions() {
             return Err(anyhow!("unsupported pax_global_extensions from tar header"));
         } else if entry_type.is_contiguous() {
             return Err(anyhow!("unsupported contiguous entry type from tar header"));
@@ -455,7 +458,7 @@ impl<'a> TarballTreeBuilder<'a> {
         if let Some(parent_path) = path.as_ref().parent() {
             if !self.path_inode_map.contains_key(parent_path) {
                 self.make_lost_dirs(parent_path, nodes)?;
-                let node = self.create_directory(parent_path, nodes)?;
+                let node = self.create_directory(parent_path, nodes.len())?;
                 nodes.push(node);
             }
         }
@@ -463,7 +466,7 @@ impl<'a> TarballTreeBuilder<'a> {
         Ok(())
     }
 
-    fn create_directory(&mut self, path: &Path, nodes: &[Node]) -> Result<Node> {
+    fn create_directory(&mut self, path: &Path, nodes_index: usize) -> Result<Node> {
         let ino = (self.path_inode_map.len() + 1) as Inode;
         let name = Self::get_file_name(path)?;
         let mut inode = InodeWrapper::new(self.ctx.fs_version);
@@ -502,7 +505,7 @@ impl<'a> TarballTreeBuilder<'a> {
         };
 
         self.path_inode_map
-            .insert(path.to_path_buf(), (ino, nodes.len()));
+            .insert(path.to_path_buf(), (ino, nodes_index));
 
         Ok(node)
     }
@@ -530,11 +533,13 @@ impl<'a> TarballTreeBuilder<'a> {
     }
 }
 
-pub(crate) struct TarballBuilder {
+/// Builder to create RAFS filesystems from tarballs.
+pub struct TarballBuilder {
     ty: ConversionType,
 }
 
 impl TarballBuilder {
+    /// Create a new instance of [TarballBuilder] to build a RAFS filesystem from a tarball.
     pub fn new(conversion_type: ConversionType) -> Self {
         Self {
             ty: conversion_type,
diff --git a/src/bin/nydus-image/main.rs b/src/bin/nydus-image/main.rs
index 10917e26d83..df0ce2d4d3d 100644
--- a/src/bin/nydus-image/main.rs
+++ b/src/bin/nydus-image/main.rs
@@ -869,10 +869,9 @@ impl Command {
         )?;
 
         // Some operations like listing xattr pairs of certain namespace need the process
-        // to be privileged. Therefore, trace what euid and egid are
+        // to be privileged. Therefore, trace what euid and egid are.
         event_tracer!("euid", "{}", geteuid());
         event_tracer!("egid", "{}", getegid());
-
         info!("successfully built RAFS filesystem: \n{}", build_output);
         OutputSerializer::dump(matches, build_output, build_info)
     }
@@ -1284,9 +1283,11 @@ impl Command {
                 }
             }
             Some(v) => {
-                let param = v.trim_start_matches("0x").trim_start_matches("0X");
-                let chunk_size =
-                    u32::from_str_radix(param, 16).context(format!("invalid chunk size {}", v))?;
+                let chunk_size = if v.starts_with("0x") || v.starts_with("0X") {
+                    u32::from_str_radix(&v[2..], 16).context(format!("invalid chunk size {}", v))?
+                } else {
+                    u32::from_str_radix(v, 10).context(format!("invalid chunk size {}", v))?
+                };
                 if chunk_size as u64 > RAFS_MAX_CHUNK_SIZE
                     || chunk_size < 0x1000
                     || !chunk_size.is_power_of_two()