Re: patch rfc: debuginfod -Z (generalized archive) support

[Mark Wielaard <mark at klomp dot org>]

Hi Frank,

On Fri, 2020-02-07 at 08:57 -0500, Frank Ch. Eigler wrote:
> > Yes, lets make =CMD optional. But why assume "cat"? Can't you then
> > simply use fopen instaed of popen in the code to get the FILE* to pass
> > to archive_read_open_FILE? Or even just open and archive_read_open_fd?
> 
> One reason: the defer_dtor<>-based auto-closing of these objects is
> different.  (popens must be closed with pclose not something else.)  A
> robust auto-closer is needed because of all the exceptions we may
> trigger.

My C++ is pretty terrible, but couldn't you handle that with something
like:

diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 6d729023..907b8fc5 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -1160,11 +1160,24 @@ handle_buildid_r_match (int64_t b_mtime,
         archive_extension = arch.first;
         archive_decoder = arch.second;
       }
-  string popen_cmd = archive_decoder + " " + shell_escape(b_source0);
-  FILE* fp = popen (popen_cmd.c_str(), "r"); // "e" O_CLOEXEC?
-  if (fp == NULL)
-    throw libc_exception (errno, string("popen ") + popen_cmd);
-  defer_dtor<FILE*,int> fp_closer (fp, pclose);
+  FILE* fp;
+  defer_dtor<FILE*,int>::dtor_fn dfn;
+  if (archive_decoder != "cat")
+    {
+      string popen_cmd = archive_decoder + " " + shell_escape(b_source0);
+      fp = popen (popen_cmd.c_str(), "r"); // "e" O_CLOEXEC?
+      dfn = pclose;
+      if (fp == NULL)
+        throw libc_exception (errno, string("popen ") + popen_cmd);
+    }
+  else
+    {
+      fp = fopen (b_source0.c_str(), "r");
+      dfn = fclose;
+      if (fp == NULL)
+        throw libc_exception (errno, string("fopen ") + b_source0);
+    }
+  defer_dtor<FILE*,int> fp_closer (fp, dfn);
 
   struct archive *a;
   a = archive_read_new();

Cheers,

Mark