From: Michael Schroeder <mls@suse.de>
Date: Fri, 2 Nov 2012 17:38:12 +0000 (+0100)
Subject: make FILE objects autoclose the fd in the destructor
X-Git-Tag: BASE-SuSE-Code-12_3-Branch~191
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=60db387f9f9843f57d79566df4033a480b6f71bb;p=thirdparty%2Flibsolv.git

make FILE objects autoclose the fd in the destructor

Makes the code much nicer for perl/python, does not help much
for ruby. Also, we have to do a glibc hack to implement a close
method needed for ruby. Sigh.
---

diff --git a/bindings/solv.i b/bindings/solv.i
index f7d6e7c4..04bb30a1 100644
--- a/bindings/solv.i
+++ b/bindings/solv.i
@@ -338,6 +338,13 @@ typedef VALUE AppObjectPtr;
 #include <sys/types.h>
 #include <unistd.h>
 
+#ifdef __GLIBC__
+/* glibc's way of closing the file without freeing the file pointer */
+extern int _IO_file_close_it(FILE *);
+#else
+#define _IO_file_close_it(fp) 0
+#endif
+
 /* argh, swig undefs bool for perl */
 #ifndef bool
 typedef int bool;
@@ -589,24 +596,19 @@ typedef struct {
 } Chksum;
 
 %rename(xfopen) solv_xfopen;
-%rename(xfopen_fd) solv_xfopen_fd;
-%rename(xfopen_dup) solv_xfopen_dup;
-%rename(xfclose) solv_xfclose;
-%rename(xfileno) solv_xfileno;
+%rename(xfopen_fd) solv_xfopen_dup;
+
+%nodefaultctor FILE;
+typedef struct {
+} FILE;
+
+%newobject solv_xfopen;
+%newobject solv_xfopen_dup;
 
 FILE *solv_xfopen(const char *fn, const char *mode = 0);
-FILE *solv_xfopen_fd(const char *fn, int fd, const char *mode = 0);
 FILE *solv_xfopen_dup(const char *fn, int fd, const char *mode = 0);
-int solv_xfclose(FILE *fp);
-int solv_xfileno(FILE *fp);
 
 %{
-  SWIGINTERN int solv_xfclose(FILE *fp) {
-    return fclose(fp);
-  }
-  SWIGINTERN int solv_xfileno(FILE *fp) {
-    return fileno(fp);
-  }
   SWIGINTERN FILE *solv_xfopen_dup(const char *fn, int fd, const char *mode) {
     fd = dup(fd);
     return fd == -1 ? 0 : solv_xfopen_fd(fn, fd, mode);
@@ -646,6 +648,26 @@ typedef struct {
   int const count;
 } TransactionClass;
 
+%extend FILE {
+  ~FILE() {
+    fclose($self);
+  }
+  int fileno() {
+    return fileno($self);
+  }
+  int dup() {
+    return dup(fileno($self));
+  }
+  bool flush() {
+    return fflush($self) == 0;
+  }
+  bool close() {
+    bool ret;
+    ret = fflush($self) == 0;
+    _IO_file_close_it($self);
+    return ret;
+  }
+}
 
 %extend Job {
   static const Id SOLVER_SOLVABLE = SOLVER_SOLVABLE;
diff --git a/examples/p5solv b/examples/p5solv
index ebade1d1..c3a994c6 100755
--- a/examples/p5solv
+++ b/examples/p5solv
@@ -109,9 +109,9 @@ sub download {
     }
   }
   if ($uncompress) {
-    return solv::xfopen_dup($file, fileno($f));
+    return solv::xfopen_fd($file, fileno($f));
   } else {
-    return solv::xfopen_dup(undef, fileno($f));
+    return solv::xfopen_fd(undef, fileno($f));
   }
 }
 
@@ -131,14 +131,12 @@ sub usecachedrepo {
       return undef if sysread($f, $fextcookie, 32) != 32;
     }
     sysseek($f, 0, Fcntl::SEEK_SET);
-    my $fd = solv::xfopen_dup(undef, fileno($f));
+    my $fd = solv::xfopen_fd(undef, fileno($f));
     my $flags = $ext ? $solv::Repo::REPO_USE_LOADING|$solv::Repo::REPO_EXTEND_SOLVABLES : 0;
     $flags |= $solv::Repo::REPO_LOCALPOOL if $ext && $ext ne 'DL';
     if (!$self->{'handle'}->add_solv($fd, $flags)) {
-      solv::xfclose($fd);
       return undef;
     }
-    solv::xfclose($fd);
     $self->{'cookie'} = $fcookie unless $ext;
     $self->{'extcookie'} = $fextcookie if $fextcookie;
     utime undef, undef, $f if $mark;
@@ -157,7 +155,7 @@ sub writecachedrepo {
   };
   return unless $f;
   chmod 0444, $f;
-  my $ff = solv::xfopen_dup(undef, fileno($f));
+  my $ff = solv::xfopen_fd(undef, fileno($f));
   if (!$info) {
     $self->{'handle'}->write($ff);
   } elsif ($ext) {
@@ -165,7 +163,7 @@ sub writecachedrepo {
   } else {
      $self->{'handle'}->write_first_repodata($ff);
   }
-  solv::xfclose($ff);
+  undef $ff;	# also flushes
   if ($self->{'type'} ne 'system' && !$ext) {
     $self->{'extcookie'} ||= $self->calc_cookie_ext($f, $self->{'cookie'});
     syswrite($f, $self->{'extcookie'});
@@ -182,7 +180,6 @@ sub writecachedrepo {
 	$info->extend_to_repo();
 	$info->add_solv($f, $solv::Repo::REPO_EXTEND_SOLVABLES);
       }
-      solv::xfclose($f);
     }
   }
   rename($tmpname, $self->cachepath($ext));
@@ -269,7 +266,6 @@ sub load_ext {
   } elsif ($ext eq 'FL') {
     $self->{'handle'}->add_deltainfoxml($f, $solv::Repo::REPO_USE_LOADING);
   }
-  solv::xfclose($f);
   $self->writecachedrepo($ext, $repodata);
   return 1;
 }
@@ -289,18 +285,15 @@ sub load {
   $self->{'cookie'} = $self->calc_cookie_fp($f);
   if ($self->usecachedrepo(undef, 1)) {
     print "cached\n";
-    solv::xfclose($f);
     return 1;
   }
   $self->{'handle'}->add_repomdxml($f, 0);
-  solv::xfclose($f);
   print "fetching\n";
   my ($filename, $filechksum) = $self->find('primary');
   if ($filename) {
     $f = $self->download($filename, 1, $filechksum, 1);
     if ($f) {
       $self->{'handle'}->add_rpmmd($f, undef, 0);
-      solv::xfclose($f);
     }
     return undef if $self->{'incomplete'};
   }
@@ -309,7 +302,6 @@ sub load {
     $f = $self->download($filename, 1, $filechksum, 1);
     if ($f) {
       $self->{'handle'}->add_updateinfoxml($f, 0);
-      solv::xfclose($f);
     }
   }
   $self->add_exts();
@@ -399,7 +391,6 @@ sub load_ext {
   my $f = $self->download("$descrdir/$filename", 1, $filechksum);
   return 0 unless $f;
   $self->{'handle'}->add_susetags($f, $defvendorid, $ext, $solv::Repo::REPO_USE_LOADING|$solv::Repo::REPO_EXTEND_SOLVABLES);
-  solv::xfclose($f);
   $self->writecachedrepo($ext, $repodata);
   return 1;
 }
@@ -419,11 +410,9 @@ sub load {
   $self->{'cookie'} = $self->calc_cookie_fp($f);
   if ($self->usecachedrepo(undef, 1)) {
     print "cached\n";
-    solv::xfclose($f);
     return 1;
   }
   $self->{'handle'}->add_content($f, 0);
-  solv::xfclose($f);
   print "fetching\n";
   my $defvendorid = $self->{'handle'}->lookup_id($solv::SOLVID_META, $solv::SUSETAGS_DEFAULTVENDOR);
   my $descrdir = $self->{'handle'}->lookup_str($solv::SOLVID_META, $solv::SUSETAGS_DESCRDIR) || 'suse/setup/descr'; 
@@ -433,14 +422,12 @@ sub load {
     $f = $self->download("$descrdir/$filename", 1, $filechksum, 1);
     if ($f) {
       $self->{'handle'}->add_susetags($f, $defvendorid, undef, $solv::Repo::REPO_NO_INTERNALIZE|$solv::Repo::SUSETAGS_RECORD_SHARES);
-      solv::xfclose($f);
       ($filename, $filechksum) = $self->find('packages.en.gz');
       ($filename, $filechksum) = $self->find('packages.en') unless $filename;
       if ($filename) {
 	$f = $self->download("$descrdir/$filename", 1, $filechksum, 1);
 	if ($f) {
 	  $self->{'handle'}->add_susetags($f, $defvendorid, undef, $solv::Repo::REPO_NO_INTERNALIZE|$solv::Repo::REPO_REUSE_REPODATA|$solv::Repo::REPO_EXTEND_SOLVABLES);
-	  solv::xfclose($f);
 	}
       }
       $self->{'handle'}->internalize();
@@ -747,8 +734,7 @@ if ($cmd eq 'install' || $cmd eq 'erase' || $cmd eq 'up' || $cmd eq 'dup' || $cm
       print "install ".$p->str()."\n";
       my $f = $newpkgsfps{$p->{'id'}};
       my $mode = $steptype == $solv::Transaction::SOLVER_TRANSACTION_INSTALL ? '-U' : '-i';
-      system('rpm', $mode, '--force', '--nodeps', '--nodigest', '--nosignature', "/dev/fd/".solv::xfileno($f)) && die("rpm failed: $?\n");
-      solv::xfclose($f);
+      system('rpm', $mode, '--force', '--nodeps', '--nodigest', '--nosignature', "/dev/fd/".$f->fileno()) && die("rpm failed: $?\n");
       delete $newpkgsfps{$p->{'id'}};
     }
   }
diff --git a/examples/pysolv b/examples/pysolv
index 75da2924..23e14c4e 100755
--- a/examples/pysolv
+++ b/examples/pysolv
@@ -112,11 +112,10 @@ class repo_generic(dict):
         self['baseurl'] = url
 
     def setfrommetalink(self, metalink):
-        nf = self.download(metalink, False, None)
-        if not nf:
+        f = self.download(metalink, False, None)
+        if not f:
             return None
-        f = os.fdopen(os.dup(solv.xfileno(nf)), 'r')
-        solv.xfclose(nf)
+        f = os.fdopen(f.dup(), 'r')
         urls = []
         chksum = None
         for l in f.readlines():
@@ -134,11 +133,10 @@ class repo_generic(dict):
         return chksum
         
     def setfrommirrorlist(self, mirrorlist):
-        nf = self.download(mirrorlist, False, None)
-        if not nf:
+        f = self.download(mirrorlist, False, None)
+        if not f:
             return
-        f = os.fdopen(os.dup(solv.xfileno(nf)), 'r')
-        solv.xfclose(nf)
+        f = os.fdopen(f.dup(), 'r')
         urls = []
         for l in f.readline():
             l = l.strip()
@@ -191,8 +189,8 @@ class repo_generic(dict):
                     self['incomplete'] = True
                 return None
         if uncompress:
-            return solv.xfopen_fd(file, os.dup(f.fileno()))
-        return solv.xfopen_fd(None, os.dup(f.fileno()))
+            return solv.xfopen_fd(file, f.fileno())
+        return solv.xfopen_fd(None, f.fileno())
 
     def usecachedrepo(self, ext, mark=False):
         try: 
@@ -273,7 +271,6 @@ class repo_generic(dict):
                     info.extend_to_repo()
                     # LOCALPOOL does not help as pool already contains all ids
                     info.add_solv(nf, Repo.REPO_EXTEND_SOLVABLES)
-                solv.xfclose(nf)
             os.rename(tmpname, self.cachepath(ext))
         except IOError, e:
             if tmpname:
@@ -315,17 +312,14 @@ class repo_repomd(repo_generic):
         self['cookie'] = self.calc_cookie_fp(f)
         if self.usecachedrepo(None, True):
             print "cached"
-            solv.xfclose(f)
             return True
         self.handle.add_repomdxml(f, 0)
-        solv.xfclose(f)
         print "fetching"
         (filename, filechksum) = self.find('primary')
         if filename:
             f = self.download(filename, True, filechksum, True)
             if f:
                 self.handle.add_rpmmd(f, None, 0)
-                solv.xfclose(f)
             if 'incomplete' in self:
                 return False # hopeless, need good primary
         (filename, filechksum) = self.find('updateinfo')
@@ -333,7 +327,6 @@ class repo_repomd(repo_generic):
             f = self.download(filename, True, filechksum, True)
             if f:
                 self.handle.add_updateinfoxml(f, 0)
-                solv.xfclose(f)
         self.add_exts()
         self.writecachedrepo(None)
         # must be called after writing the repo
@@ -403,7 +396,6 @@ class repo_repomd(repo_generic):
             self.handle.add_rpmmd(f, 'FL', Repo.REPO_USE_LOADING|Repo.REPO_EXTEND_SOLVABLES)
         elif ext == 'DL':
             self.handle.add_deltainfoxml(f, Repo.REPO_USE_LOADING)
-        solv.xfclose(f)
         self.writecachedrepo(ext, repodata)
         return True
 
@@ -422,10 +414,8 @@ class repo_susetags(repo_generic):
         self['cookie'] = self.calc_cookie_fp(f)
         if self.usecachedrepo(None, True):
             print "cached"
-            solv.xfclose(f)
             return True
         self.handle.add_content(f, 0)
-        solv.xfclose(f)
         print "fetching"
         defvendorid = self.handle.lookup_id(solv.SOLVID_META, solv.SUSETAGS_DEFAULTVENDOR)
         descrdir = self.handle.lookup_str(solv.SOLVID_META, solv.SUSETAGS_DESCRDIR)
@@ -438,7 +428,6 @@ class repo_susetags(repo_generic):
             f = self.download(descrdir + '/' + filename, True, filechksum, True)
             if f:
                 self.handle.add_susetags(f, defvendorid, None, Repo.REPO_NO_INTERNALIZE|Repo.SUSETAGS_RECORD_SHARES)
-                solv.xfclose(f)
                 (filename, filechksum) = self.find('packages.en.gz')
                 if not filename:
                     (filename, filechksum) = self.find('packages.en')
@@ -446,7 +435,6 @@ class repo_susetags(repo_generic):
                     f = self.download(descrdir + '/' + filename, True, filechksum, True)
                     if f:
                         self.handle.add_susetags(f, defvendorid, None, Repo.REPO_NO_INTERNALIZE|Repo.REPO_REUSE_REPODATA|Repo.REPO_EXTEND_SOLVABLES)
-                        solv.xfclose(f)
                 self.handle.internalize()
         self.add_exts()
         self.writecachedrepo(None)
@@ -530,7 +518,6 @@ class repo_susetags(repo_generic):
         if not f:
             return False
         self.handle.add_susetags(f, defvendorid, ext, Repo.REPO_USE_LOADING|Repo.REPO_EXTEND_SOLVABLES)
-        solv.xfclose(f)
         self.writecachedrepo(ext, repodata)
         return True
 
@@ -887,11 +874,11 @@ if cmd == 'install' or cmd == 'erase' or cmd == 'up' or cmd == 'dup' or cmd == '
                     if not f:
                         continue
                     nf = tempfile.TemporaryFile()
-                    nf = os.dup(nf.fileno())
-                    st = subprocess.call(['/usr/bin/applydeltarpm', '-a', p.arch, "/dev/fd/%d" % solv.xfileno(f), "/dev/fd/%d" % nf])
-                    solv.xfclose(f)
+                    nf = os.dup(nf.fileno())   # get rid of CLOEXEC
+                    st = subprocess.call(['/usr/bin/applydeltarpm', '-a', p.arch, "/dev/fd/%d" % f.fileno(), "/dev/fd/%d" % nf])
                     os.lseek(nf, 0, os.SEEK_SET)
                     newpkgsfp[p.id] = solv.xfopen_fd("", nf)
+                    os.close(nf)
                     break
                 if p.id in newpkgsfp:
                     sys.stdout.write("d")
diff --git a/examples/rbsolv b/examples/rbsolv
index eb773955..9f75fafe 100755
--- a/examples/rbsolv
+++ b/examples/rbsolv
@@ -104,11 +104,14 @@ class Repo_generic
 	return nil
       end
     end
+    rf = nil
     if uncompress
-      return Solv::xfopen_dup(file, f.fileno)
+      rf = Solv::xfopen_fd(file, f.fileno)
     else
-      return Solv::xfopen_dup('', f.fileno)
+      rf = Solv::xfopen_fd('', f.fileno)
     end
+    f.close
+    return rf
   end
 
   def usecachedrepo(ext, mark = false)
@@ -126,14 +129,15 @@ class Repo_generic
 	return false if fextcookie.length != 32
       end
       f.sysseek(0, IO::SEEK_SET)
-      f = Solv::xfopen_dup('', f.fileno)
+      nf = Solv::xfopen_fd('', f.fileno)
+      f.close
       flags = ext ? Solv::Repo::REPO_USE_LOADING|Solv::Repo::REPO_EXTEND_SOLVABLES : 0
       flags |= Solv::Repo::REPO_LOCALPOOL if ext && ext != 'DL'
-      if ! @handle.add_solv(f, flags)
-	Solv::xfclose(f)
+      if ! @handle.add_solv(nf, flags)
+	nf.close
 	return false
       end
-      Solv::xfclose(f)
+      nf.close()
       @cookie = fcookie unless ext
       @extcookie = fextcookie if !ext && @type != 'system'
       now = Time.now
@@ -154,7 +158,7 @@ class Repo_generic
       Dir::mkdir("/var/cache/solv", 0755) unless FileTest.directory?("/var/cache/solv")
       f =  Tempfile.new('.newsolv-', '/var/cache/solv')
       f.chmod(0444)
-      sf = Solv::xfopen_dup('', f.fileno)
+      sf = Solv::xfopen_fd('', f.fileno)
       if !info
 	@handle.write(sf)
       elsif ext
@@ -162,14 +166,14 @@ class Repo_generic
       else
 	@handle.write_first_repodata(sf)
       end
-      Solv::xfclose(sf)
+      sf.close
       f.sysseek(0, IO::SEEK_END)
       if @type != 'system' && !ext
 	@extcookie = calc_cookie_ext(f, @cookie) unless @extcookie
 	f.syswrite(@extcookie)
       end
       f.syswrite(ext ? @extcookie : @cookie)
-      f.close(false)
+      f.close
       if @handle.iscontiguous?
 	sf = Solv::xfopen(f.path)
 	if sf
@@ -180,7 +184,7 @@ class Repo_generic
 	    info.extend_to_repo()
 	    info.add_solv(sf, Solv::Repo::REPO_EXTEND_SOLVABLES)
 	  end
-	  Solv::xfclose(sf)
+	  sf.close
 	end
       end
       File.rename(f.path, cachepath(ext))
@@ -242,18 +246,18 @@ class Repo_rpmmd < Repo_generic
     @cookie = calc_cookie_fp(f)
     if usecachedrepo(nil, true)
       puts "cached"
-      Solv.xfclose(f)
+      f.close
       return true
     end
     @handle.add_repomdxml(f, 0)
-    Solv::xfclose(f)
+    f.close
     puts "fetching"
     filename, filechksum = find('primary')
     if filename
       f = download(filename, true, filechksum, true)
       if f
 	@handle.add_rpmmd(f, nil, 0)
-	Solv::xfclose(f)
+	f.close
       end
       return false if @incomplete
     end
@@ -262,7 +266,7 @@ class Repo_rpmmd < Repo_generic
       f = download(filename, true, filechksum, true)
       if f
 	@handle.add_updateinfoxml(f, 0)
-	Solv::xfclose(f)
+	f.close
       end
     end
     add_exts()
@@ -321,7 +325,7 @@ class Repo_rpmmd < Repo_generic
     elsif ext == 'DL'
       @handle.add_deltainfoxml(f, Solv::Repo::REPO_USE_LOADING)
     end
-    Solv::xfclose(f)
+    f.close
     writecachedrepo(ext, repodata)
     return true
   end
@@ -354,11 +358,11 @@ class Repo_susetags < Repo_generic
     @cookie = calc_cookie_fp(f)
     if usecachedrepo(nil, true)
       puts "cached"
-      Solv.xfclose(f)
+      f.close
       return true
     end
     @handle.add_content(f, 0)
-    Solv::xfclose(f)
+    f.close
     puts "fetching"
     defvendorid = @handle.lookup_id(Solv::SOLVID_META, Solv::SUSETAGS_DEFAULTVENDOR)
     descrdir = @handle.lookup_str(Solv::SOLVID_META, Solv::SUSETAGS_DESCRDIR)
@@ -369,14 +373,14 @@ class Repo_susetags < Repo_generic
       f = download("#{descrdir}/#{filename}", true, filechksum, true)
       if f
 	@handle.add_susetags(f, defvendorid, nil, Solv::Repo::REPO_NO_INTERNALIZE|Solv::Repo::SUSETAGS_RECORD_SHARES)
-	Solv::xfclose(f)
+	f.close
 	(filename, filechksum) = find('packages.en.gz')
 	(filename, filechksum) = find('packages.en') unless filename
 	if filename
 	  f = download("#{descrdir}/#{filename}", true, filechksum, true)
 	  if f
 	    @handle.add_susetags(f, defvendorid, nil, Solv::Repo::REPO_NO_INTERNALIZE|Solv::Repo::REPO_REUSE_REPODATA|Solv::Repo::REPO_EXTEND_SOLVABLES)
-	    Solv::xfclose(f)
+	    f.close
 	  end
 	end
 	@handle.internalize()
@@ -450,7 +454,7 @@ class Repo_susetags < Repo_generic
     f = download("#{descrdir}/#{filename}", true, filechksum)
     return false unless f
     @handle.add_susetags(f, defvendorid, ext, Solv::Repo::REPO_USE_LOADING|Solv::Repo::REPO_EXTEND_SOLVABLES)
-    Solv::xfclose(f)
+    f.close
     writecachedrepo(ext, repodata)
     return true
   end
@@ -747,8 +751,8 @@ if cmd == 'install' || cmd == 'erase' || cmd == 'up' || cmd == 'dup' || cmd == '
       f = newpkgsfp.delete(p.id)
       next unless f
       mode = steptype == Solv::Transaction::SOLVER_TRANSACTION_INSTALL ? '-U' : '-i'
-      system('rpm', mode, '--force', '--nodeps', '--nodigest', '--nosignature', "/dev/fd/#{Solv::xfileno(f).to_s}") || abort("rpm failed: #{$? >> 8}")
-      solv::xfclose(f)
+      system('rpm', mode, '--force', '--nodeps', '--nodigest', '--nosignature', "/dev/fd/#{f.fileno().to_s}") || abort("rpm failed: #{$? >> 8}")
+      f.close
     end
   end
 end