Solved some pr comments

oss-internship-2020/libarchive/README.md

@@ -4,8 +4,8 @@ Sandboxed version of the [libarchive](https://www.libarchive.org/) minitar [exam
 
 ## Build
 
-First, run `git submodule update --init --recursive` to update submodules.
-After this, run the following commands:
+<!-- First, run `git submodule update --init --recursive` to update submodules.
+After this, run the following commands: -->
 
 `mkdir -p build && cd build`
 
@@ -17,7 +17,8 @@ After this, run the following commands:
 The example binary file can be found at `build/examples/sapi_minitar` and the unit tests at `build/test/sapi_minitar_test`.
 
 ## Patches
-TODO
+
+The original libarchive code required patching since one of the custom types produced errors with libclang Python errors. The patches are applied automatically during the build step and they do not modify the functionality of the library. The repository is also fetched automatically.
 
 ## Examples
 
@@ -49,5 +50,3 @@ The available options are:
 - *z* - Compress with GZIP.
 
 If no compression method is chosen (in the case of archive creation) the files will only be archived.
-
-

oss-internship-2020/libarchive/examples/CMakeLists.txt

@@ -19,14 +19,14 @@ add_library(sapi_minitar_lib STATIC
 )
 
 target_link_libraries(sapi_minitar_lib PUBLIC
-  libarchive_sapi
-  sapi::sapi
-  sandbox2::fileops
-  sandbox2::util
-  sandbox2::file_base
-  sandbox2::executor
-  sandbox2::temp_file
   glog::glog
+  libarchive_sapi
+  sandbox2::executor
+  sandbox2::fileops
+  sandbox2::file_base
+  sandbox2::util
+  sandbox2::temp_file
+  sapi::sapi
 )
 
 target_include_directories(sapi_minitar_lib INTERFACE
@@ -39,4 +39,4 @@ add_executable(sapi_minitar
 
 target_link_libraries(sapi_minitar PRIVATE
   sapi_minitar_lib
-)
+)

oss-internship-2020/libarchive/examples/sandbox.h

@@ -12,8 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#ifndef SAPI_LIBARCHIVE_SANDBOX_H
-#define SAPI_LIBARCHIVE_SANDBOX_H
+#ifndef SAPI_LIBARCHIVE_EXAMPLES_SANDBOX_H
+#define SAPI_LIBARCHIVE_EXAMPLES_SANDBOX_H
 
 #include <asm/unistd_64.h>
 
@@ -65,7 +65,7 @@ class SapiLibarchiveSandboxCreate : public LibarchiveSandbox {
     // We check whether the entry is a file or a directory.
     for (const auto& i : files_) {
       struct stat s;
-      stat(i.c_str(), &s);
+      CHECK(stat(i.c_str(), &s) == 0) << "Could not stat " << i;
       if (S_ISDIR(s.st_mode)) {
         policy = policy.AddDirectory(i);
       } else {
@@ -100,7 +100,7 @@ class SapiLibarchiveSandboxExtract : public LibarchiveSandbox {
     // If the user only wants to list the entries in the archive, we do
     // not need to worry about changing directories;
     if (do_extract_) {
-      executor = &executor->set_cwd(std::string(tmp_dir_));
+      executor->set_cwd(std::string(tmp_dir_));
     }
   }
 
@@ -141,4 +141,4 @@ class SapiLibarchiveSandboxExtract : public LibarchiveSandbox {
   const int do_extract_;
 };
 
-#endif  // SAPI_LIBARCHIVE_SANDBOX_H
+#endif  // SAPI_LIBARCHIVE_EXAMPLES_SANDBOX_H

oss-internship-2020/libarchive/examples/sapi_minitar.cc

@@ -13,17 +13,17 @@
 // limitations under the License.
 
 #include "sapi_minitar.h"
+#include "sandboxed_api/sandbox2/util/path.h"
 
 void create(const char* initial_filename, int compress, const char** argv,
-            bool verbose /* = true */) {
+            bool verbose) {
   // We split the filename path into dirname and filename. To the filename we
   // prepend "/output/"" so that it will work with the security policy.
   std::string abs_path = MakeAbsolutePathAtCWD(std::string(initial_filename));
   auto [archive_path, filename_tmp] =
       std::move(sandbox2::file::SplitPath(abs_path));
 
-  std::string filename("/output/");
-  filename.append(filename_tmp);
+  std::string filename = sandbox2::file::JoinPath("/output/", filename_tmp);
 
   std::vector<std::string> absolute_paths;
   sandbox2::util::CharPtrArrToVecString(const_cast<char* const*>(argv),
@@ -259,7 +259,7 @@ void create(const char* initial_filename, int compress, const char** argv,
 }
 
 void extract(const char* filename, int do_extract, int flags,
-             bool verbose /* = true */) {
+             bool verbose) {
   std::string tmp_dir;
   if (do_extract) {
     tmp_dir = CreateTempDirAtCWD();
@@ -268,18 +268,19 @@ void extract(const char* filename, int do_extract, int flags,
   // We can use a struct like this in order to delete the temporary
   // directory that was created earlier whenever the function ends.
   struct ExtractTempDirectoryCleanup {
+    ExtractTempDirectoryCleanup(const std::string& dir): dir_(dir) {}
     ~ExtractTempDirectoryCleanup() {
-      sandbox2::file_util::fileops::DeleteRecursively(dir);
+      sandbox2::file_util::fileops::DeleteRecursively(dir_);
     }
-    std::string dir;
+   private:
+    std::string dir_;
   };
 
   // We should only delete it if the do_extract flag is true which
   // means that this struct is instantiated only in that case.
   std::unique_ptr<ExtractTempDirectoryCleanup> cleanup_ptr;
   if (do_extract) {
-    cleanup_ptr = absl::make_unique<ExtractTempDirectoryCleanup>();
-    cleanup_ptr->dir = tmp_dir;
+    cleanup_ptr = absl::make_unique<ExtractTempDirectoryCleanup>(tmp_dir);
   }
 
   std::string filename_absolute = MakeAbsolutePathAtCWD(filename);