wrap every unlink call to check for != /dev/null

Unlinking /dev/null is bad, we shouldn't do that. Also, we should print
at least a warning if we tried to unlink a file but didn't manage to
pull it of (ignoring the case were the file is /dev/null or doesn't
exist in the first place).

This got triggered by a relatively unlikely to cause problem in
pkgAcquire::Worker::PrepareFiles which would while temporary
uncompressed files (which are set to keep compressed) figure out that to
files are the same and prepare for sharing by deleting them. Bad move.
That also shows why not printing a warning is a bad idea as this hide
the error for in non-root test runs.

Git-Dch: Ignore

apt-pkg/acquire-item.cc

@@ -147,20 +147,6 @@ static bool BootstrapPDiffWith(std::string const &PartialFile, std::string const
    return typeItr != types.cend();
 }
 									/*}}}*/
-static bool RemoveFile(char const * const Function, std::string const &FileName)/*{{{*/
-{
-   if (FileName == "/dev/null")
-      return true;
-   errno = 0;
-   if (unlink(FileName.c_str()) != 0)
-   {
-      if (errno == ENOENT)
-	 return true;
-      return _error->WarningE(Function, "Removal of file %s failed", FileName.c_str());
-   }
-   return true;
-}
-									/*}}}*/
 
 static bool MessageInsecureRepository(bool const isError, std::string const &msg)/*{{{*/
 {

apt-pkg/acquire-worker.cc

@@ -742,9 +742,9 @@ void pkgAcquire::Worker::PrepareFiles(char const * const caller, pkgAcquire::Que
       for (pkgAcquire::Queue::QItem::owner_iterator O = Itm->Owners.begin(); O != Itm->Owners.end(); ++O)
       {
 	 pkgAcquire::Item const * const Owner = *O;
-	 if (Owner->DestFile == filename)
+	 if (Owner->DestFile == filename || filename == "/dev/null")
 	    continue;
-	 unlink(Owner->DestFile.c_str());
+	 RemoveFile("PrepareFiles", Owner->DestFile);
 	 if (link(filename.c_str(), Owner->DestFile.c_str()) != 0)
 	 {
 	    // different mounts can't happen for us as we download to lists/ by default,
@@ -759,7 +759,7 @@ void pkgAcquire::Worker::PrepareFiles(char const * const caller, pkgAcquire::Que
    else
    {
       for (pkgAcquire::Queue::QItem::owner_iterator O = Itm->Owners.begin(); O != Itm->Owners.end(); ++O)
-	 unlink((*O)->DestFile.c_str());
+	 RemoveFile("PrepareFiles", (*O)->DestFile);
    }
 }
 									/*}}}*/

apt-pkg/acquire.cc

@@ -645,7 +645,7 @@ bool pkgAcquire::Clean(string Dir)
       
       // Nothing found, nuke it
       if (I == Items.end())
-	 unlink(Dir->d_name);
+	 RemoveFile("Clean", Dir->d_name);
    };
    
    closedir(D);
@@ -993,15 +993,15 @@ void pkgAcquire::Queue::QItem::SyncDestinationFiles() const		/*{{{*/
       if (lstat((*O)->DestFile.c_str(),&file) == 0)
       {
 	 if ((file.st_mode & S_IFREG) == 0)
-	    unlink((*O)->DestFile.c_str());
+	    RemoveFile("SyncDestinationFiles", (*O)->DestFile);
 	 else if (supersize < file.st_size)
 	 {
 	    supersize = file.st_size;
-	    unlink(superfile.c_str());
+	    RemoveFile("SyncDestinationFiles", superfile);
 	    rename((*O)->DestFile.c_str(), superfile.c_str());
 	 }
 	 else
-	    unlink((*O)->DestFile.c_str());
+	    RemoveFile("SyncDestinationFiles", (*O)->DestFile);
 	 if (symlink(superfile.c_str(), (*O)->DestFile.c_str()) != 0)
 	 {
 	    ; // not a problem per-se and no real alternative

apt-pkg/cachefile.cc

@@ -191,9 +191,9 @@ void pkgCacheFile::RemoveCaches()
    std::string const srcpkgcache = _config->FindFile("Dir::cache::srcpkgcache");
 
    if (pkgcache.empty() == false && RealFileExists(pkgcache) == true)
-      unlink(pkgcache.c_str());
+      RemoveFile("RemoveCaches", pkgcache);
    if (srcpkgcache.empty() == false && RealFileExists(srcpkgcache) == true)
-      unlink(srcpkgcache.c_str());
+      RemoveFile("RemoveCaches", srcpkgcache);
    if (pkgcache.empty() == false)
    {
       std::string cachedir = flNotFile(pkgcache);
@@ -207,7 +207,7 @@ void pkgCacheFile::RemoveCaches()
 	    std::string nuke = flNotDir(*file);
 	    if (strncmp(cachefile.c_str(), nuke.c_str(), cachefile.length()) != 0)
 	       continue;
-	    unlink(file->c_str());
+	    RemoveFile("RemoveCaches", *file);
 	 }
       }
    }
@@ -226,7 +226,7 @@ void pkgCacheFile::RemoveCaches()
       std::string nuke = flNotDir(*file);
       if (strncmp(cachefile.c_str(), nuke.c_str(), cachefile.length()) != 0)
 	 continue;
-      unlink(file->c_str());
+      RemoveFile("RemoveCaches", *file);
    }
 }
 									/*}}}*/

apt-pkg/cdrom.cc

@@ -426,8 +426,8 @@ bool pkgCdrom::WriteDatabase(Configuration &Cnf)
 {
    string DFile = _config->FindFile("Dir::State::cdroms");
    string NewFile = DFile + ".new";
-   
-   unlink(NewFile.c_str());
+
+   RemoveFile("WriteDatabase", NewFile);
    ofstream Out(NewFile.c_str());
    if (!Out)
       return _error->Errno("ofstream::ofstream",
@@ -468,7 +468,7 @@ bool pkgCdrom::WriteSourceList(string Name,vector<string> &List,bool Source)
       return _error->Errno("ifstream::ifstream","Opening %s",File.c_str());
 
    string NewFile = File + ".new";
-   unlink(NewFile.c_str());
+   RemoveFile("WriteDatabase", NewFile);
    ofstream Out(NewFile.c_str());
    if (!Out)
       return _error->Errno("ofstream::ofstream",

apt-pkg/contrib/fileutl.cc

@@ -169,6 +169,21 @@ bool CopyFile(FileFd &From,FileFd &To)
 	 return false;
    } while (ToRead != 0);
 
+   return true;
+}
+									/*}}}*/
+bool RemoveFile(char const * const Function, std::string const &FileName)/*{{{*/
+{
+   if (FileName == "/dev/null")
+      return true;
+   errno = 0;
+   if (unlink(FileName.c_str()) != 0)
+   {
+      if (errno == ENOENT)
+	 return true;
+
+      return _error->WarningE(Function,_("Problem unlinking the file %s"), FileName.c_str());
+   }
    return true;
 }
 									/*}}}*/
@@ -1135,13 +1150,13 @@ bool FileFd::Open(string FileName,unsigned int const Mode,APT::Configuration::Co
    else if ((OpenMode & (Exclusive | Create)) == (Exclusive | Create))
    {
       // for atomic, this will be done by rename in Close()
-      unlink(FileName.c_str());
+      RemoveFile("FileFd::Open", FileName);
    }
    if ((OpenMode & Empty) == Empty)
    {
       struct stat Buf;
       if (lstat(FileName.c_str(),&Buf) == 0 && S_ISLNK(Buf.st_mode))
-	 unlink(FileName.c_str());
+	 RemoveFile("FileFd::Open", FileName);
    }
 
    int fileflags = 0;
@@ -2025,8 +2040,7 @@ bool FileFd::Close()
 
    if ((Flags & Fail) == Fail && (Flags & DelOnFail) == DelOnFail &&
        FileName.empty() == false)
-      if (unlink(FileName.c_str()) != 0)
-	 Res &= _error->WarningE("unlnk",_("Problem unlinking the file %s"), FileName.c_str());
+      Res &= RemoveFile("FileFd::Close", FileName);
 
    if (Res == false)
       Flags |= Fail;

apt-pkg/contrib/fileutl.h

@@ -150,6 +150,7 @@ class FileFd
 
 bool RunScripts(const char *Cnf);
 bool CopyFile(FileFd &From,FileFd &To);
+bool RemoveFile(char const * const Function, std::string const &FileName);
 int GetLock(std::string File,bool Errors = true);
 bool FileExists(std::string File);
 bool RealFileExists(std::string File);

apt-pkg/deb/dpkgpm.cc

@@ -1597,7 +1597,7 @@ bool pkgDPkgPM::Go(APT::Progress::PackageManager *progress)
    {
       std::string const oldpkgcache = _config->FindFile("Dir::cache::pkgcache");
       if (oldpkgcache.empty() == false && RealFileExists(oldpkgcache) == true &&
-	  unlink(oldpkgcache.c_str()) == 0)
+	  RemoveFile("pkgDPkgPM::Go", oldpkgcache))
       {
 	 std::string const srcpkgcache = _config->FindFile("Dir::cache::srcpkgcache");
 	 if (srcpkgcache.empty() == false && RealFileExists(srcpkgcache) == true)

apt-pkg/edsp/edsplistparser.cc

@@ -31,12 +31,10 @@ public:
    edspListParserPrivate()
    {
       std::string const states = _config->FindFile("Dir::State::extended_states");
-      if (states != "/dev/null")
-	 unlink(states.c_str());
+      RemoveFile("edspListParserPrivate", states);
       extendedstates.Open(states, FileFd::WriteOnly | FileFd::Create | FileFd::Exclusive, 0600);
       std::string const prefs = _config->FindFile("Dir::Etc::preferences");
-      if (prefs != "/dev/null")
-	 unlink(prefs.c_str());
+      RemoveFile("edspListParserPrivate", prefs);
       preferences.Open(prefs, FileFd::WriteOnly | FileFd::Create | FileFd::Exclusive, 0600);
    }
 };

apt-pkg/edsp/edspsystem.cc

@@ -57,8 +57,8 @@ public:
       if (tempDir.empty())
 	 return;
 
-      unlink(tempStatesFile.c_str());
-      unlink(tempPrefsFile.c_str());
+      RemoveFile("DeInitialize", tempStatesFile);
+      RemoveFile("DeInitialize", tempPrefsFile);
       rmdir(tempDir.c_str());
    }
 

apt-private/private-download.cc

@@ -331,7 +331,7 @@ bool DoClean(CommandLine &)
 	 c1out << "Del " << Pkg << " " << Ver << " [" << SizeToStr(St.st_size) << "B]" << std::endl;
 
 	 if (_config->FindB("APT::Get::Simulate") == false)
-	    unlink(File);
+	    RemoveFile("Cleaner::Erase", File);
       };
 };
 bool DoAutoClean(CommandLine &)

cmdline/apt-dump-solver.cc

@@ -53,8 +53,7 @@ int main(int argc,const char *argv[])					/*{{{*/
 	   return 0;
 	}
 
-	if (strcmp(filename, "/dev/null") != 0)
-	   unlink(filename);
+	RemoveFile(argv[0], filename);
 	FileFd input, output;
 	if (input.OpenDescriptor(STDIN_FILENO, FileFd::ReadOnly) == false ||
 	      output.Open(filename, FileFd::WriteOnly | FileFd::Create | FileFd::Exclusive, 0600) == false ||

ftparchive/byhash.cc

@@ -41,9 +41,8 @@ void DeleteAllButMostRecent(std::string dir, int KeepFiles)
    auto files = GetListOfFilesInDir(dir, false);
    std::sort(files.begin(), files.end(), Cmp());
 
-   for (auto I=files.begin(); I<files.end()-KeepFiles; I++) {
-      unlink((*I).c_str());
-   }
+   for (auto I=files.begin(); I<files.end()-KeepFiles; ++I)
+      RemoveFile("DeleteAllButMostRecent", *I);
 }
 
 // Takes a input filename (e.g. binary-i386/Packages) and a hashstring

ftparchive/multicompress.cc

@@ -352,9 +352,7 @@ bool MultiCompress::Child(int const &FD)
 	 for (Files *I = Outputs; I != 0; I = I->Next)
 	 {
 	    I->TmpFile.Close();
-	    if (unlink(I->TmpFile.Name().c_str()) != 0)
-	       _error->Errno("unlink",_("Problem unlinking %s"),
-			     I->TmpFile.Name().c_str());
+	    RemoveFile("MultiCompress::Child", I->TmpFile.Name());
 	 }
 	 return !_error->PendingError();
       }      

ftparchive/writer.cc

@@ -302,9 +302,7 @@ bool FTWScanner::Delink(string &FileName,const char *OriginalPath,
 	       _error->Errno("readlink",_("Failed to readlink %s"),OriginalPath);
 	    else
 	    {
-	       if (unlink(OriginalPath) != 0)
-		  _error->Errno("unlink",_("Failed to unlink %s"),OriginalPath);
-	       else
+	       if (RemoveFile("FTWScanner::Delink", OriginalPath))
 	       {
 		  if (link(FileName.c_str(),OriginalPath) != 0)
 		  {

methods/file.cc

@@ -76,7 +76,7 @@ bool FileMethod::Fetch(FetchItem *Itm)
       }
    }
    if (Res.IMSHit != true)
-      unlink(Itm->DestFile.c_str());
+      RemoveFile("file", Itm->DestFile);
 
    // See if the file exists
    if (stat(File.c_str(),&Buf) == 0)

methods/ftp.cc

@@ -1090,7 +1090,7 @@ bool FtpMethod::Fetch(FetchItem *Itm)
 	 // If the file is missing we hard fail and delete the destfile
 	 // otherwise transient fail
 	 if (Missing == true) {
-	    unlink(FailFile.c_str());
+	    RemoveFile("ftp", FailFile);
 	    return false;
 	 }
 	 Fail(true);

methods/https.cc

@@ -443,7 +443,7 @@ bool HttpsMethod::Fetch(FetchItem *Itm)
    // server says file not modified
    if (Server->Result == 304 || curl_condition_unmet == 1)
    {
-      unlink(File->Name().c_str());
+      RemoveFile("https", File->Name());
       Res.IMSHit = true;
       Res.LastModified = Itm->LastModified;
       Res.Size = 0;
@@ -461,14 +461,14 @@ bool HttpsMethod::Fetch(FetchItem *Itm)
       SetFailReason(err);
       _error->Error("%i %s", Server->Result, Server->Code);
       // unlink, no need keep 401/404 page content in partial/
-      unlink(File->Name().c_str());
+      RemoveFile("https", File->Name());
       return false;
    }
 
    // invalid range-request
    if (Server->Result == 416)
    {
-      unlink(File->Name().c_str());
+      RemoveFile("https", File->Name());
       delete File;
       Redirect(Itm->Uri);
       return true;

methods/mirror.cc

@@ -122,7 +122,7 @@ bool MirrorMethod::Clean(string Dir)
       }
       // nothing found, nuke it
       if (I == list.end())
-	 unlink(Dir->d_name);
+	 RemoveFile("mirror", Dir->d_name);
    }
 
    closedir(D);

methods/server.cc

@@ -274,7 +274,7 @@ ServerMethod::DealWithHeaders(FetchResult &Res)
    // Not Modified
    if (Server->Result == 304)
    {
-      unlink(Queue->DestFile.c_str());
+      RemoveFile("server", Queue->DestFile);
       Res.IMSHit = true;
       Res.LastModified = Queue->LastModified;
       return IMS_HIT;
@@ -350,7 +350,7 @@ ServerMethod::DealWithHeaders(FetchResult &Res)
 	    Server->StartPos = Server->TotalFileSize;
 	    Server->Result = 200;
 	 }
-	 else if (unlink(Queue->DestFile.c_str()) == 0)
+	 else if (RemoveFile("server", Queue->DestFile))
 	 {
 	    NextURI = Queue->Uri;
 	    return TRY_AGAIN_OR_REDIRECT;