[sheepdog] [PATCH RFC] sheep: remove assert() for better error messages
Hitoshi Mitake
h.mitake at gmail.com
Mon Oct 1 15:34:25 CEST 2012
This patch removes direct usage of assert() in sheep. assert() in
daemon process causes problem because assert() writes its error
message to stderr. In addition, typical daemon processes including
sheep close stderr and dup() other fd. Actually, logger process of
sheep dup() /dev/null for stderr.
>From my experience, using assert() directly in daemon processes makes
debugging hard. So this patch adds wrappers of assert() and
daemon(). With this patch, assert() can find suitable fd for error
messages and the error messages can appear in a log file of sheep.
Signed-off-by: Hitoshi Mitake <h.mitake at gmail.com>
---
include/daemonize.h | 10 ++++++++++
include/sd_assert.h | 31 +++++++++++++++++++++++++++++++
include/strbuf.h | 2 +-
lib/Makefile.am | 2 +-
lib/daemonize.c | 19 +++++++++++++++++++
sheep/cluster/accord.c | 2 +-
sheep/cluster/local.c | 2 +-
sheep/cluster/zookeeper.c | 2 +-
sheep/group.c | 2 +-
sheep/request.c | 2 +-
sheep/sheep.c | 4 +++-
sheep/trace/graph.c | 2 +-
12 files changed, 71 insertions(+), 9 deletions(-)
create mode 100644 include/daemonize.h
create mode 100644 include/sd_assert.h
create mode 100644 lib/daemonize.c
diff --git a/include/daemonize.h b/include/daemonize.h
new file mode 100644
index 0000000..57924e5
--- /dev/null
+++ b/include/daemonize.h
@@ -0,0 +1,10 @@
+
+#ifndef DAEMONIZE_H
+#define DAEMONIZE_H
+
+#include <stdbool.h>
+
+bool is_daemonized(void);
+int daemonize(void);
+
+#endif /* DAEMONIZE_H */
diff --git a/include/sd_assert.h b/include/sd_assert.h
new file mode 100644
index 0000000..05c553e
--- /dev/null
+++ b/include/sd_assert.h
@@ -0,0 +1,31 @@
+#ifndef SD_ASSERT_H
+#define SD_ASSERT_H
+
+#include "logger.h"
+#include "daemonize.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#ifdef assert
+#error "don't include assert.h directly"
+#endif
+
+#define assert(expr) do { \
+ if (expr) \
+ break; \
+ \
+ if (is_daemonized()) { \
+ vprintf(SDOG_ERR, "assert: %s:%d: %s: " \
+ "Asserting `%s' failed.\n", \
+ __FILE__, __LINE__, __func__, #expr); \
+ } else { \
+ fprintf(stderr, "assert: %s:%d: %s: " \
+ "Asserting `%s' failed.\n", \
+ __FILE__, __LINE__, __func__, #expr); \
+ } \
+ \
+ abort(); \
+ } while (0)
+
+#endif /* SD_ASSERT_H */
diff --git a/include/strbuf.h b/include/strbuf.h
index 42794b9..c785d34 100644
--- a/include/strbuf.h
+++ b/include/strbuf.h
@@ -1,7 +1,6 @@
#ifndef STRBUF_H
#define STRBUF_H
-#include <assert.h>
#include <ctype.h>
#include <stdarg.h>
#include <unistd.h>
@@ -9,6 +8,7 @@
#include <stdlib.h>
#include "util.h"
+#include "sd_assert.h"
struct strbuf {
size_t alloc;
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 1c962fd..9020a33 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -4,4 +4,4 @@ INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include
noinst_LIBRARIES = libsheepdog.a
-libsheepdog_a_SOURCES = event.c logger.c net.c util.c rbtree.c strbuf.c sha1.c
+libsheepdog_a_SOURCES = event.c logger.c net.c util.c rbtree.c strbuf.c sha1.c daemonize.c
diff --git a/lib/daemonize.c b/lib/daemonize.c
new file mode 100644
index 0000000..c4f747b
--- /dev/null
+++ b/lib/daemonize.c
@@ -0,0 +1,19 @@
+
+#include <unistd.h>
+#include <stdbool.h>
+
+static bool daemonized;
+
+bool is_daemonized(void)
+{
+ return daemonized;
+}
+
+int daemonize(void)
+{
+ if (daemon(0, 0))
+ return -1;
+
+ daemonized = true;
+ return 0;
+}
diff --git a/sheep/cluster/accord.c b/sheep/cluster/accord.c
index a0f1d00..a75213a 100644
--- a/sheep/cluster/accord.c
+++ b/sheep/cluster/accord.c
@@ -12,7 +12,6 @@
#include <string.h>
#include <unistd.h>
#include <search.h>
-#include <assert.h>
#include <pthread.h>
#include <sys/epoll.h>
#include <sys/eventfd.h>
@@ -21,6 +20,7 @@
#include "cluster.h"
#include "event.h"
#include "work.h"
+#include "sd_assert.h"
#define BASE_FILE "/sheepdog"
#define LOCK_FILE BASE_FILE "/lock"
diff --git a/sheep/cluster/local.c b/sheep/cluster/local.c
index c7ef38e..0a6fb0f 100644
--- a/sheep/cluster/local.c
+++ b/sheep/cluster/local.c
@@ -17,11 +17,11 @@
#include <sys/file.h>
#include <signal.h>
#include <fcntl.h>
-#include <assert.h>
#include "cluster.h"
#include "event.h"
#include "work.h"
+#include "sd_assert.h"
#define MAX_EVENTS 500
#define PROCESS_CHECK_INTERVAL 200 /* ms */
diff --git a/sheep/cluster/zookeeper.c b/sheep/cluster/zookeeper.c
index 4cd5be8..911b5d1 100644
--- a/sheep/cluster/zookeeper.c
+++ b/sheep/cluster/zookeeper.c
@@ -12,7 +12,6 @@
#include <string.h>
#include <unistd.h>
#include <search.h>
-#include <assert.h>
#include <sys/epoll.h>
#include <sys/eventfd.h>
#include <zookeeper/zookeeper.h>
@@ -21,6 +20,7 @@
#include "cluster.h"
#include "event.h"
#include "work.h"
+#include "sd_assert.h"
#define SESSION_TIMEOUT 30000 /* millisecond */
#define MEMBER_CREATE_TIMEOUT SESSION_TIMEOUT
diff --git a/sheep/group.c b/sheep/group.c
index 7ec3523..79aa66c 100644
--- a/sheep/group.c
+++ b/sheep/group.c
@@ -8,7 +8,6 @@
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <assert.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
@@ -28,6 +27,7 @@
#include "logger.h"
#include "work.h"
#include "cluster.h"
+#include "sd_assert.h"
struct node {
struct sd_node ent;
diff --git a/sheep/request.c b/sheep/request.c
index 40a01d5..288f0be 100644
--- a/sheep/request.c
+++ b/sheep/request.c
@@ -8,7 +8,6 @@
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <assert.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
@@ -21,6 +20,7 @@
#include <fcntl.h>
#include "sheep_priv.h"
+#include "sd_assert.h"
static void requeue_request(struct request *req);
diff --git a/sheep/sheep.c b/sheep/sheep.c
index cf42205..fbcadb0 100644
--- a/sheep/sheep.c
+++ b/sheep/sheep.c
@@ -30,6 +30,8 @@
#include "sheep_priv.h"
#include "trace/trace.h"
+#include "daemonize.h"
+#include "sd_assert.h"
#define EPOLL_SIZE 4096
#define DEFAULT_OBJECT_DIR "/tmp"
@@ -454,7 +456,7 @@ int main(int argc, char **argv)
srandom(port);
- if (is_daemon && daemon(0, 0))
+ if (is_daemon && daemonize())
exit(1);
ret = init_base_path(dir);
diff --git a/sheep/trace/graph.c b/sheep/trace/graph.c
index ab3a6c3..7e916e0 100644
--- a/sheep/trace/graph.c
+++ b/sheep/trace/graph.c
@@ -12,12 +12,12 @@
*/
#include <time.h>
-#include <assert.h>
#include <sched.h>
#include "trace.h"
#include "logger.h"
#include "util.h"
+#include "sd_assert.h"
static __thread int ret_stack_index;
static __thread struct trace_ret_stack {
--
1.7.5.1
More information about the sheepdog
mailing list