BufferedFileFdPrivate: Make InternalFlush() save against errors

[apt.git] / apt-pkg / contrib / fileutl.cc

diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc
index aa5831f583501e3711cf64b2962e57692549fc60..7f3ed673a13144e7563a7c8e136d57371c98415a 100644 (file)
--- a/apt-pkg/contrib/fileutl.cc
+++ b/apt-pkg/contrib/fileutl.cc
@@ -919,40 +919,89 @@ bool ChangeOwnerAndPermissionOfFile(char const * const requester, char const * c
 }
                                                                        /*}}}*/
 
 }
                                                                        /*}}}*/
 

+struct APT_HIDDEN simple_buffer {                                                      /*{{{*/
+   static constexpr size_t buffersize_max = 4096;
+   unsigned long long bufferstart = 0;
+   unsigned long long bufferend = 0;
+   char buffer[buffersize_max];
+
+   const char *get() const { return buffer + bufferstart; }
+   char *get() { return buffer + bufferstart; }
+   bool empty() const { return bufferend <= bufferstart; }
+   bool full() const { return bufferend == buffersize_max; }
+   unsigned long long size() const { return bufferend-bufferstart; }
+   void reset() { bufferend = bufferstart = 0; }
+   ssize_t read(void *to, unsigned long long requested_size) APT_MUSTCHECK
+   {
+      if (size() < requested_size)
+        requested_size = size();
+      memcpy(to, buffer + bufferstart, requested_size);
+      bufferstart += requested_size;
+      if (bufferstart == bufferend)
+        bufferstart = bufferend = 0;
+      return requested_size;
+   }
+   ssize_t write(const void *from, unsigned long long requested_size) APT_MUSTCHECK
+   {
+      if (buffersize_max - size() < requested_size)
+        requested_size = buffersize_max - size();
+      memcpy(buffer + bufferend, from, requested_size);
+      bufferend += requested_size;
+      if (bufferstart == bufferend)
+        bufferstart = bufferend = 0;
+      return requested_size;
+   }
+};
+                                                                       /*}}}*/
+

 class APT_HIDDEN FileFdPrivate {                                                       /*{{{*/
 class APT_HIDDEN FileFdPrivate {                                                       /*{{{*/

+   friend class BufferedWriteFileFdPrivate;

 protected:
    FileFd * const filefd;
 protected:
    FileFd * const filefd;

-   struct simple_buffer {
-         static constexpr size_t buffersize_max = 4096;
-        unsigned long long bufferstart = 0;
-        unsigned long long bufferend = 0;
-        char buffer[buffersize_max];
-
-        char *get() { return buffer + bufferstart; }
-        bool empty() { return bufferend <= bufferstart; }
-        unsigned long long size() { return bufferend-bufferstart; }
-        void reset() { bufferend = bufferstart = 0; }
-        ssize_t read(void *to, unsigned long long requested_size) APT_MUSTCHECK
-        {
-           if (size() < requested_size)
-              requested_size = size();
-           memcpy(to, buffer + bufferstart, requested_size);
-           bufferstart += requested_size;
-           if (bufferstart == bufferend)
-              bufferstart = bufferend = 0;
-           return requested_size;
-        }
-   } buffer;
-public:
+   simple_buffer buffer;

    int compressed_fd;
    pid_t compressor_pid;
    bool is_pipe;
    APT::Configuration::Compressor compressor;
    unsigned int openmode;
    unsigned long long seekpos;
    int compressed_fd;
    pid_t compressor_pid;
    bool is_pipe;
    APT::Configuration::Compressor compressor;
    unsigned int openmode;
    unsigned long long seekpos;

+public:
+

    explicit FileFdPrivate(FileFd * const pfilefd) : filefd(pfilefd),
       compressed_fd(-1), compressor_pid(-1), is_pipe(false),
       openmode(0), seekpos(0) {};
    explicit FileFdPrivate(FileFd * const pfilefd) : filefd(pfilefd),
       compressed_fd(-1), compressor_pid(-1), is_pipe(false),
       openmode(0), seekpos(0) {};

+   virtual APT::Configuration::Compressor get_compressor() const
+   {
+      return compressor;
+   }
+   virtual void set_compressor(APT::Configuration::Compressor const &compressor)
+   {
+      this->compressor = compressor;
+   }
+   virtual unsigned int get_openmode() const
+   {
+      return openmode;
+   }
+   virtual void set_openmode(unsigned int openmode)
+   {
+      this->openmode = openmode;
+   }
+   virtual bool get_is_pipe() const
+   {
+      return is_pipe;
+   }
+   virtual void set_is_pipe(bool is_pipe)
+   {
+      this->is_pipe = is_pipe;
+   }
+   virtual unsigned long long get_seekpos() const
+   {
+      return seekpos;
+   }
+   virtual void set_seekpos(unsigned long long seekpos)
+   {
+      this->seekpos = seekpos;
+   }

 
    virtual bool InternalOpen(int const iFd, unsigned int const Mode) = 0;
    ssize_t InternalRead(void * To, unsigned long long Size)
 
    virtual bool InternalOpen(int const iFd, unsigned int const Mode) = 0;
    ssize_t InternalRead(void * To, unsigned long long Size)


@@ -970,13 +1019,12 @@ public:
    {
       if (unlikely(Size == 0))
         return nullptr;
    {
       if (unlikely(Size == 0))
         return nullptr;

+      // Read one byte less than buffer size to have space for trailing 0.

       --Size;
       --Size;

-      To[0] = '\0';
-      if (unlikely(Size == 0))
-        return To;
+

       char * const InitialTo = To;
 
       char * const InitialTo = To;
 

-      do {
+      while (Size > 0) {

         if (buffer.empty() == true)
         {
            buffer.reset();
         if (buffer.empty() == true)
         {
            buffer.reset();


@@ -1004,10 +1052,14 @@ public:
         Size -= actualread;
         if (newline != nullptr)
            break;
         Size -= actualread;
         if (newline != nullptr)
            break;

-      } while (Size > 0);
+      }

       *To = '\0';
       return InitialTo;
    }
       *To = '\0';
       return InitialTo;
    }

+   virtual bool InternalFlush()
+   {
+      return true;
+   }

    virtual ssize_t InternalWrite(void const * const From, unsigned long long const Size) = 0;
    virtual bool InternalWriteError() { return filefd->FileFdErrno("write",_("Write error")); }
    virtual bool InternalSeek(unsigned long long const To)
    virtual ssize_t InternalWrite(void const * const From, unsigned long long const Size) = 0;
    virtual bool InternalWriteError() { return filefd->FileFdErrno("write",_("Write error")); }
    virtual bool InternalSeek(unsigned long long const To)


@@ -1098,6 +1150,152 @@ public:
    virtual ~FileFdPrivate() {}
 };
                                                                        /*}}}*/
    virtual ~FileFdPrivate() {}
 };
                                                                        /*}}}*/

+class APT_HIDDEN BufferedWriteFileFdPrivate : public FileFdPrivate {                                                   /*{{{*/
+protected:
+   FileFdPrivate *wrapped;
+   simple_buffer writebuffer;
+
+public:
+
+   explicit BufferedWriteFileFdPrivate(FileFdPrivate *Priv) :
+      FileFdPrivate(Priv->filefd), wrapped(Priv) {};
+
+   virtual APT::Configuration::Compressor get_compressor() const override
+   {
+      return wrapped->get_compressor();
+   }
+   virtual void set_compressor(APT::Configuration::Compressor const &compressor)  override
+   {
+      return wrapped->set_compressor(compressor);
+   }
+   virtual unsigned int get_openmode() const  override
+   {
+      return wrapped->get_openmode();
+   }
+   virtual void set_openmode(unsigned int openmode)  override
+   {
+      return wrapped->set_openmode(openmode);
+   }
+   virtual bool get_is_pipe() const  override
+   {
+      return wrapped->get_is_pipe();
+   }
+   virtual void set_is_pipe(bool is_pipe) override
+   {
+      FileFdPrivate::set_is_pipe(is_pipe);
+      wrapped->set_is_pipe(is_pipe);
+   }
+   virtual unsigned long long get_seekpos() const override
+   {
+      return wrapped->get_seekpos();
+   }
+   virtual void set_seekpos(unsigned long long seekpos) override
+   {
+      return wrapped->set_seekpos(seekpos);
+   }
+   virtual bool InternalOpen(int const iFd, unsigned int const Mode) override
+   {
+      if (InternalFlush() == false)
+        return false;
+      return wrapped->InternalOpen(iFd, Mode);
+   }
+   virtual ssize_t InternalUnbufferedRead(void * const To, unsigned long long const Size) override
+   {
+      if (InternalFlush() == false)
+        return -1;
+      return wrapped->InternalUnbufferedRead(To, Size);
+
+   }
+   virtual bool InternalReadError() override
+   {
+      return wrapped->InternalReadError();
+   }
+   virtual char * InternalReadLine(char * To, unsigned long long Size) override
+   {
+      if (InternalFlush() == false)
+        return nullptr;
+      return wrapped->InternalReadLine(To, Size);
+   }
+   virtual bool InternalFlush() override
+   {
+      while (writebuffer.empty() == false) {
+        auto written = wrapped->InternalWrite(writebuffer.get(),
+                                              writebuffer.size());
+        // Ignore interrupted syscalls
+        if (written < 0 && errno == EINTR)
+           continue;
+        if (written < 0)
+           return false;
+
+        writebuffer.bufferstart += written;
+      }
+
+      writebuffer.reset();
+      return true;
+   }
+   virtual ssize_t InternalWrite(void const * const From, unsigned long long const Size) override
+   {
+      size_t written = 0;
+
+      while (written < Size) {
+        auto buffered = writebuffer.write(static_cast<char const*>(From) + written, Size - written);
+
+        written += buffered;
+
+        if (writebuffer.full() && InternalFlush() == false)
+              return -1;
+      }
+
+      return written;
+   }
+   virtual bool InternalWriteError()
+   {
+      return wrapped->InternalWriteError();
+   }
+   virtual bool InternalSeek(unsigned long long const To)
+   {
+      if (InternalFlush() == false)
+        return false;
+      return wrapped->InternalSeek(To);
+   }
+   virtual bool InternalSkip(unsigned long long Over)
+   {
+      if (InternalFlush() == false)
+        return false;
+      return wrapped->InternalSkip(Over);
+   }
+   virtual bool InternalTruncate(unsigned long long const Size)
+   {
+      if (InternalFlush() == false)
+        return false;
+      return wrapped->InternalTruncate(Size);
+   }
+   virtual unsigned long long InternalTell()
+   {
+      if (InternalFlush() == false)
+        return -1;
+      return wrapped->InternalTell();
+   }
+   virtual unsigned long long InternalSize()
+   {
+      if (InternalFlush() == false)
+        return -1;
+      return wrapped->InternalSize();
+   }
+   virtual bool InternalClose(std::string const &FileName)
+   {
+      return wrapped->InternalClose(FileName);
+   }
+   virtual bool InternalAlwaysAutoClose() const
+   {
+      return wrapped->InternalAlwaysAutoClose();
+   }
+   virtual ~BufferedWriteFileFdPrivate()
+   {
+      delete wrapped;
+   }
+};
+                                                                       /*}}}*/

 class APT_HIDDEN GzipFileFdPrivate: public FileFdPrivate {                             /*{{{*/
 #ifdef HAVE_ZLIB
 public:
 class APT_HIDDEN GzipFileFdPrivate: public FileFdPrivate {                             /*{{{*/
 #ifdef HAVE_ZLIB
 public:


@@ -1512,7 +1710,7 @@ public:
         SetCloseExec(Pipe[J],true);
 
       compressed_fd = filefd->iFd;
         SetCloseExec(Pipe[J],true);
 
       compressed_fd = filefd->iFd;

-      is_pipe = true;
+      set_is_pipe(true);

 
       if (Comp == true)
         filefd->iFd = Pipe[1];
 
       if (Comp == true)
         filefd->iFd = Pipe[1];


@@ -1934,8 +2132,11 @@ bool FileFd::OpenInternDescriptor(unsigned int const Mode, APT::Configuration::C
       else
         d = new PipedFileFdPrivate(this);
 
       else
         d = new PipedFileFdPrivate(this);
 

-      d->openmode = Mode;
-      d->compressor = compressor;
+      if (Mode & BufferedWrite)
+        d = new BufferedWriteFileFdPrivate(d);
+
+      d->set_openmode(Mode);
+      d->set_compressor(compressor);

       if ((Flags & AutoClose) != AutoClose && d->InternalAlwaysAutoClose())
       {
         // Need to duplicate fd here or gz/bz2 close for cleanup will close the fd as well
       if ((Flags & AutoClose) != AutoClose && d->InternalAlwaysAutoClose())
       {
         // Need to duplicate fd here or gz/bz2 close for cleanup will close the fd as well


@@ -1993,7 +2194,7 @@ bool FileFd::Read(void *To,unsigned long long Size,unsigned long long *Actual)
       To = (char *)To + Res;
       Size -= Res;
       if (d != NULL)
       To = (char *)To + Res;
       Size -= Res;
       if (d != NULL)

-        d->seekpos += Res;
+        d->set_seekpos(d->get_seekpos() + Res);

       if (Actual != 0)
         *Actual += Res;
    }
       if (Actual != 0)
         *Actual += Res;
    }


@@ -2023,6 +2224,15 @@ char* FileFd::ReadLine(char *To, unsigned long long const Size)
    return d->InternalReadLine(To, Size);
 }
                                                                        /*}}}*/
    return d->InternalReadLine(To, Size);
 }
                                                                        /*}}}*/

+// FileFd::Flush - Flush the file                                      /*{{{*/
+bool FileFd::Flush()
+{
+   if (d == nullptr)
+      return true;
+
+   return d->InternalFlush();
+}
+                                                                       /*}}}*/

 // FileFd::Write - Write to the file                                   /*{{{*/
 bool FileFd::Write(const void *From,unsigned long long Size)
 {
 // FileFd::Write - Write to the file                                   /*{{{*/
 bool FileFd::Write(const void *From,unsigned long long Size)
 {


@@ -2041,7 +2251,7 @@ bool FileFd::Write(const void *From,unsigned long long Size)
       From = (char const *)From + Res;
       Size -= Res;
       if (d != NULL)
       From = (char const *)From + Res;
       Size -= Res;
       if (d != NULL)

-        d->seekpos += Res;
+        d->set_seekpos(d->get_seekpos() + Res);

    }
 
    if (Size == 0)
    }
 
    if (Size == 0)


@@ -2109,13 +2319,13 @@ unsigned long long FileFd::Tell()
    off_t const Res = d->InternalTell();
    if (Res == (off_t)-1)
       FileFdErrno("lseek","Failed to determine the current file position");
    off_t const Res = d->InternalTell();
    if (Res == (off_t)-1)
       FileFdErrno("lseek","Failed to determine the current file position");

-   d->seekpos = Res;
+   d->set_seekpos(Res);

    return Res;
 }
                                                                        /*}}}*/
 static bool StatFileFd(char const * const msg, int const iFd, std::string const &FileName, struct stat &Buf, FileFdPrivate * const d) /*{{{*/
 {
    return Res;
 }
                                                                        /*}}}*/
 static bool StatFileFd(char const * const msg, int const iFd, std::string const &FileName, struct stat &Buf, FileFdPrivate * const d) /*{{{*/
 {

-   bool ispipe = (d != NULL && d->is_pipe == true);
+   bool ispipe = (d != NULL && d->get_is_pipe() == true);

    if (ispipe == false)
    {
       if (fstat(iFd,&Buf) != 0)
    if (ispipe == false)
    {
       if (fstat(iFd,&Buf) != 0)


@@ -2132,7 +2342,7 @@ static bool StatFileFd(char const * const msg, int const iFd, std::string const
       // we set it here, too, as we get the info here for free
       // in theory the Open-methods should take care of it already
       if (d != NULL)
       // we set it here, too, as we get the info here for free
       // in theory the Open-methods should take care of it already
       if (d != NULL)

-        d->is_pipe = true;
+        d->set_is_pipe(true);

       if (stat(FileName.c_str(), &Buf) != 0)
         return _error->Errno("fstat", "Unable to determine %s for file %s", msg, FileName.c_str());
    }
       if (stat(FileName.c_str(), &Buf) != 0)
         return _error->Errno("fstat", "Unable to determine %s for file %s", msg, FileName.c_str());
    }


@@ -2176,6 +2386,8 @@ unsigned long long FileFd::Size()
 /* */
 bool FileFd::Close()
 {
 /* */
 bool FileFd::Close()
 {

+   if (Flush() == false)
+      return false;

    if (iFd == -1)
       return true;
 
    if (iFd == -1)
       return true;