[llvm-objdump] Add RAII for xar apis

[fjricci]

Summary

xar_open and xar_iter_new require manual calls to close/free functions
to deallocate resources. This makes it easy to introduce memory leaks,
so add RAII struct wrappers for these resources.

[fjricci]

clang-format

[davide]

LGTM

[compnerd]

tools/llvm-objdump/MachODump.cpp

205 ↗

(On Diff #117885)

Can you please call this ScopedXarFile?

214 ↗

(On Diff #117885)

ScopedXarIter sounds better

[fjricci]

Xar -> ScopedXar

[alexander-shaposhnikov]

tools/llvm-objdump/MachODump.cpp

205 ↗

(On Diff #117885)

this probably should be in the anonymous namespace

[fjricci]

anonymous namespace

[alexander-shaposhnikov]

tools/llvm-objdump/MachODump.cpp

211 ↗

(On Diff #117895)

just to double check - does xar_close work fine with nulls ? (i.e. if xar_open has failed)

[fjricci]

Don't call deallocation routines on null xar objects

[compnerd]

tools/llvm-objdump/MachODump.cpp

206 ↗

(On Diff #117928)

These really should have the copy and move constructors deleted as well as the assignment operator deleted.

[fjricci]

Delete copy constructor (which implicitly deletes move constructor), and
assignment operator

[fjricci]

clang-format

[dblaikie]

llvm/trunk/tools/llvm-objdump/MachODump.cpp

206

I'd consider dropping the "Scoped" suffix & just having XarFile and XarIterator - but up to you.

222

Consider using the init list:

ScopedXarIter() : iter(xar_iter_new()) {}

(or I guess maybe even a NSDMI:

xar_iter_t iter = xar_iter_new();

)

[fjricci]

llvm/trunk/tools/llvm-objdump/MachODump.cpp

222

Used initializer list in rL315243. (Not a big fan of the NSDMI because we need to define a destructor anyway)

llvm/trunk/tools/llvm-objdump/MachODump.cpp

 };
 
 // Types for the storted data in code table that is built before disassembly
 // and the predicate function to sort them.
 typedef std::pair<uint64_t, DiceRef> DiceTableEntry;
 typedef std::vector<DiceTableEntry> DiceTable;
 typedef DiceTable::iterator dice_table_iterator;
 
+namespace {
+struct ScopedXarFile {
+  xar_t xar;
+  ScopedXarFile(const char *filename, int32_t flags) {
+    xar = xar_open(filename, flags);
+  }
+  ~ScopedXarFile() {
+    if (xar)
+      xar_close(xar);
+  }
+  ScopedXarFile(const ScopedXarFile &) = delete;
+  ScopedXarFile &operator=(const ScopedXarFile &) = delete;
+  operator xar_t() { return xar; }
+};
+
+struct ScopedXarIter {
+  xar_iter_t iter;
+  ScopedXarIter() { iter = xar_iter_new(); }
+  ~ScopedXarIter() {
+    if (iter)
+      xar_iter_free(iter);
+  }
+  ScopedXarIter(const ScopedXarIter &) = delete;
+  ScopedXarIter &operator=(const ScopedXarIter &) = delete;
+  operator xar_iter_t() { return iter; }
+};
+} // namespace
+
 // This is used to search for a data in code table entry for the PC being
 // disassembled.  The j parameter has the PC in j.first.  A single data in code
 // table entry can cover many bytes for each of its Kind's.  So if the offset,
 // aka the i.first value, of the data in code table entry plus its Length
 // covers the PC being searched for this will return true.  If not it will
 // return false.
 static bool compareDiceTableEntries(const DiceTableEntry &i,
                                     const DiceTableEntry &j) {
if(mode & S_ISVTX)
     outs() << "t";
   else if(mode & (S_IEXEC >> 6))
     outs() << "x";
   else
     outs() << "-";
 }
 
 static void PrintXarFilesSummary(const char *XarFilename, xar_t xar) {
-  xar_iter_t xi;
   xar_file_t xf;
-  xar_iter_t xp;
   const char *key, *type, *mode, *user, *group, *size, *mtime, *name, *m;
   char *endp;
   uint32_t mode_value;
 
-  xi = xar_iter_new();
+  ScopedXarIter xi;
   if (!xi) {
     errs() << "Can't obtain an xar iterator for xar archive "
            << XarFilename << "\n";
     return;
   }
 
   // Go through the xar's files.
   for (xf = xar_file_first(xar, xi); xf; xf = xar_file_next(xi)) {
-    xp = xar_iter_new();
+    ScopedXarIter xp;
     if(!xp){
       errs() << "Can't obtain an xar iterator for xar archive "
              << XarFilename << "\n";
       return;
     }
     type = nullptr;
     mode = nullptr;
     user = nullptr;
if(mtime != nullptr){
       outs() << " ";
       for( ; *m != 'Z' && *m != '\0'; m++)
         outs() << *m;
       outs() << " ";
     }
     if(name != nullptr)
       outs() << name;
     outs() << "\n";
-    xar_iter_free(xp);
   }
-  xar_iter_free(xi);
 }
 
 static void DumpBitcodeSection(MachOObjectFile *O, const char *sect,
                                 uint32_t size, bool verbose,
                                 bool PrintXarHeader, bool PrintXarFileHeaders,
                                 std::string XarMemberName) {
   if(size < sizeof(struct xar_header)) {
     outs() << "size of (__LLVM,__bundle) section too small (smaller than size "
#endif
   ToolOutputFile XarFile(XarFilename, FD);
   raw_fd_ostream &XarOut = XarFile.os();
   StringRef XarContents(sect, size);
   XarOut << XarContents;
   XarOut.close();
   if (XarOut.has_error())
     return;
 
-  xar_t xar = xar_open(XarFilename.c_str(), READ);
+  ScopedXarFile xar(XarFilename.c_str(), READ);
   if (!xar) {
     errs() << "Can't create temporary xar archive " << XarFilename << "\n";
     return;
   }
 
   SmallString<128> TocFilename;
   std::error_code TocEC =
       sys::fs::createTemporaryFile("llvm-objdump", "toc", TocFilename);
#endif
   if (!XarMemberName.empty())
     outs() << "In xar member " << XarMemberName << ": ";
   else
     outs() << "For (__LLVM,__bundle) section: ";
   outs() << "xar table of contents:\n";
   outs() << Buffer->getBuffer() << "\n";
 
   // TODO: Go through the xar's files.
-  xar_iter_t xi = xar_iter_new();
+  ScopedXarIter xi;
   if(!xi){
     errs() << "Can't obtain an xar iterator for xar archive "
            << XarFilename.c_str() << "\n";
-    xar_close(xar);
     return;
   }
   for(xar_file_t xf = xar_file_first(xar, xi); xf; xf = xar_file_next(xi)){
     const char *key;
-    xar_iter_t xp;
     const char *member_name, *member_type, *member_size_string;
     size_t member_size;
 
-    xp = xar_iter_new();
+    ScopedXarIter xp;
     if(!xp){
       errs() << "Can't obtain an xar iterator for xar archive "
              << XarFilename.c_str() << "\n";
-      xar_close(xar);
       return;
     }
     member_name = NULL;
     member_type = NULL;
     member_size_string = NULL;
     for(key = xar_prop_first(xf, xp); key; key = xar_prop_next(xp)){
       const char *val = nullptr;
       xar_prop_get(xf, key, &val);
#endif
                                  PrintXarHeader, PrintXarFileHeaders,
                                  XarMemberName);
           }
           XarMemberName = OldXarMemberName;
           delete buffer;
         }
       }
     }
-    xar_iter_free(xp);
   }
-  xar_iter_free(xi);
-  xar_close(xar);
 }
 #endif // defined(HAVE_LIBXAR)
 
 static void printObjcMetaData(MachOObjectFile *O, bool verbose) {
   if (O->is64Bit())
     printObjc2_64bit_MetaData(O, verbose);
   else {
     MachO::mach_header H;