[PATCH xorg-gtest 5/5] xserver: add RemoveLogFile()

[Peter Hutterer]

On Mon, Aug 13, 2012 at 09:32:00AM -0700, Chase Douglas wrote:
> On 08/09/2012 10:38 PM, Peter Hutterer wrote:
> >Simple unlink() call to the logfile in use. The log file is only removed if
> >the server was started and terminated successfully. If it was never started
> >or the startup failed for some reason, this function does nothing.
> >
> >Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >---
> >  include/xorg/gtest/xorg-gtest-xserver.h | 6 ++++++
> >  src/xserver.cpp                         | 5 +++++
> >  2 files changed, 11 insertions(+)
> >
> >diff --git a/include/xorg/gtest/xorg-gtest-xserver.h b/include/xorg/gtest/xorg-gtest-xserver.h
> >index f3bda9b..33446a8 100644
> >--- a/include/xorg/gtest/xorg-gtest-xserver.h
> >+++ b/include/xorg/gtest/xorg-gtest-xserver.h
> >@@ -95,6 +95,12 @@ class XServer : public xorg::testing::Process {
> >      virtual bool Kill(unsigned int timeout = 0);
> >
> >      /**
> >+     * Remove the log file used by this server. If the server was never
> >+     * started or the startup failed, this function does nothing.
> >+     */
> >+    void RemoveLogFile();
> >+
> >+    /**
> >       * Waits until this server is ready to take connections.
> >       */
> >      void WaitForConnections(void);
> >diff --git a/src/xserver.cpp b/src/xserver.cpp
> >index 86c8484..114c736 100644
> >--- a/src/xserver.cpp
> >+++ b/src/xserver.cpp
> >@@ -365,6 +365,11 @@ bool xorg::testing::XServer::Kill(unsigned int timeout) {
> >      return true;
> >  }
> >
> >+void xorg::testing::XServer::RemoveLogFile() {
> >+  if (GetState() == TERMINATED)
> >+    unlink(d_->options["-logfile"].c_str());
> >+}
> >+
> >  void xorg::testing::XServer::SetOption(const std::string &key, const std::string &value) {
> >    d_->options[key] = value;
> >  }
> >
> 
> I would prefer to call it something that gives a better idea that it
> is conditional on successful termination of the server.
> RemoveLogFile() sounds like a method that would *always* remove the
> log file.
> 
> What about "CleanUpLogFile()"?

tbh, I don't think there's much difference between the two names, and, had
we both functions, I certainly couldn't tell the difference between the two
without reading the documentation. The behaviour is stated in the API
documentation and not removing what we didn't create is a good behaviour.

> Or, you could just leave it up to the caller to call it only where
> it should be called. The caller can get the state just the same.

Without the TERMINATED condition, this would simply be an unlink call, for
any file the caller wants. If callers really just wanted to remove a file
unconditionally, they can call unlink() themselves.

Cheers,
   Peter