Re: [PATCH xorg-gtest 5/5] xserver: add RemoveLogFile()
On 08/13/2012 04:12 PM, Peter Hutterer wrote:
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.hutte...@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.
If it's not any clearer to you, then maybe there isn't a perfect
solution. I guess I'm ok with RemoveLogFile().
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.
True.
After this discussion, I'm ready to give my tag:
Reviewed-by: Chase Douglas chase.doug...@canonical.com
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xorg-gtest 5/5] xserver: add RemoveLogFile()
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.hutte...@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()?
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.
-- Chase
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xorg-gtest 5/5] xserver: add RemoveLogFile()
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.hutte...@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;
}
--
1.7.11.2
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
