Merge pull request #25 from maralla/bind-config

maralla · maralla · commit e9a754724e8c · 2016-02-26T17:46:58.000+08:00
fix bind config
diff --git a/src/corvus.c b/src/corvus.c
@@ -21,7 +21,7 @@
 
 static struct context *contexts;
 
-static void config_init()
+void config_init()
 {
     memset(config.cluster, 0, CLUSTER_NAME_SIZE + 1);
     strncpy(config.cluster, "default", CLUSTER_NAME_SIZE);
@@ -40,17 +40,18 @@ static void config_init()
     config.metric_interval = 10;
 }
 
-static int config_add(char *name, char *value)
+int config_add(char *name, char *value)
 {
     int val;
     if (strcmp(name, "cluster") == 0) {
         if (strlen(value) <= 0) return 0;
         strncpy(config.cluster, value, CLUSTER_NAME_SIZE);
     } else if (strcmp(name, "bind") == 0) {
-        config.bind = atoi(value);
-        if (config.bind > 0xFFFF) return -1;
+        if (socket_parse_port(value, &config.bind) == CORVUS_ERR) {
+            return CORVUS_ERR;
+        }
     } else if (strcmp(name, "syslog") == 0) {
-        config.syslog = atoi(value);
+        config.syslog = atoi(value) ? 1 : 0;
     } else if (strcmp(name, "thread") == 0) {
         config.thread = atoi(value);
         if (config.thread <= 0) config.thread = 4;
@@ -106,7 +107,7 @@ static int config_add(char *name, char *value)
     return 0;
 }
 
-static int read_conf(const char *filename)
+int read_conf(const char *filename)
 {
     FILE *fp = fopen(filename, "r");
     if (fp == NULL) {
@@ -137,7 +138,7 @@ static int read_conf(const char *filename)
     return 0;
 }
 
-static void quit()
+void quit()
 {
     if (config.stats) stats_kill();
 
@@ -155,7 +156,7 @@ static void quit()
     }
 }
 
-static void log_traceback()
+void log_traceback()
 {
     void *stack[64];
     char **symbols;
@@ -185,7 +186,7 @@ static void log_traceback()
     exit(EXIT_FAILURE);
 }
 
-static void sig_handler(int sig)
+void sig_handler(int sig)
 {
     switch (sig) {
         case SIGINT:
@@ -198,7 +199,7 @@ static void sig_handler(int sig)
     }
 }
 
-static void setup_signal()
+void setup_signal()
 {
     struct sigaction act;
     sigemptyset(&act.sa_mask);
@@ -224,18 +225,15 @@ struct context *get_contexts()
     return contexts;
 }
 
-void context_init(struct context *ctx, bool syslog, int log_level)
+void context_init(struct context *ctx)
 {
     memset(ctx, 0, sizeof(struct context));
 
-    ctx->syslog = syslog;
-    ctx->log_level = log_level;
     dict_init(&ctx->server_table);
     ctx->started = false;
     ctx->role = THREAD_UNKNOWN;
     ctx->state = CTX_UNKNOWN;
     mbuf_init(ctx);
-    log_init(ctx);
 
     STAILQ_INIT(&ctx->free_cmdq);
     TAILQ_INIT(&ctx->servers);
@@ -375,7 +373,7 @@ int main(int argc, const char *argv[])
 
     contexts = malloc(sizeof(struct context) * (config.thread + 1));
     for (i = 0; i <= config.thread; i++) {
-        context_init(&contexts[i], config.syslog, config.loglevel);
+        context_init(&contexts[i]);
         contexts[i].role = THREAD_UNKNOWN;
     }
 
diff --git a/src/corvus.h b/src/corvus.h
@@ -57,10 +57,6 @@ struct context {
     struct connection proxy;
     struct connection timer;
 
-    /* logging */
-    bool syslog;
-    int log_level;
-
     /* connection pool */
     struct dict server_table;
     struct conn_tqh conns;
@@ -88,7 +84,7 @@ struct {
     struct node_conf node;
     int thread;
     int loglevel;
-    int syslog;
+    bool syslog;
     char statsd_addr[DSN_MAX + 1];
     int metric_interval;
     int stats;
diff --git a/src/logging.c b/src/logging.c
@@ -10,21 +10,6 @@
 
 static const char *LEVEL_MAP[] = {"DEBUG", "INFO", "WARN", "ERROR"};
 static const int SYSLOG_LEVEL_MAP[] = {LOG_DEBUG, LOG_INFO, LOG_WARNING, LOG_ERR};
-static bool enable_syslog = false;
-static int log_level = INFO;
-
-void log_init(struct context *ctx)
-{
-    if (ctx->syslog) {
-        enable_syslog = true;
-    } else {
-        enable_syslog = false;
-    }
-
-    if (ctx->log_level != -1) {
-        log_level = ctx->log_level;
-    }
-}
 
 void logger(const char *file, int line, int level, const char *fmt, ...)
 {
@@ -33,15 +18,15 @@ void logger(const char *file, int line, int level, const char *fmt, ...)
     char timestamp[64];
     struct timeval now;
 
-    if (level < log_level) return;
+    if (level < config.loglevel) return;
 
     pid_t thread_id = (pid_t)syscall(SYS_gettid);
 
     va_start(ap, fmt);
     vsnprintf(msg, sizeof(msg), fmt, ap);
     va_end(ap);
 
-    if (enable_syslog) {
+    if (config.syslog) {
         syslog(SYSLOG_LEVEL_MAP[level], "[%d] %s", (int)thread_id, msg);
     } else {
         gettimeofday(&now, NULL);
diff --git a/src/socket.c b/src/socket.c
@@ -318,17 +318,26 @@ void socket_address_init(struct address *addr, char *host, int len, int port)
     addr->port = port;
 }
 
+int socket_parse_port(char *ptr, uint16_t *res)
+{
+    char *end;
+    int port = strtol(ptr, &end, 0);
+    if (*end != '\0' || port > 0xFFFF || port <= 0) return CORVUS_ERR;
+    *res = port;
+    return CORVUS_OK;
+}
+
 int socket_parse_addr(char *addr, struct address *address)
 {
-    unsigned long port;
-    char *colon, *end;
+    uint16_t port;
+    char *colon;
 
     colon = strchr(addr, ':');
     if (colon == NULL) return CORVUS_ERR;
 
-    port = strtoul(colon + 1, &end, 0);
-    if (*end != '\0' || end == colon + 1) return CORVUS_ERR;
-    if (port > 0xFFFF) return CORVUS_ERR;
+    if (socket_parse_port(colon + 1, &port) == CORVUS_ERR) {
+        return CORVUS_ERR;
+    }
 
     socket_address_init(address, addr, colon - addr, port);
     return port;
diff --git a/src/socket.h b/src/socket.h
@@ -30,6 +30,7 @@ void socket_address_init(struct address *addr, char *host, int len, int port);
 int socket_set_nonblocking(int fd);
 int socket_set_tcpnodelay(int fd);
 int socket_set_timeout(int fd, int timeout);
+int socket_parse_port(char *ptr, uint16_t *res);
 int socket_parse_addr(char *addr, struct address *address);
 void socket_get_key(struct address *addr, char *dst);
 
diff --git a/tests/corvus_test.c b/tests/corvus_test.c
@@ -59,6 +59,7 @@ extern TEST_CASE(test_dict);
 extern TEST_CASE(test_socket);
 extern TEST_CASE(test_client);
 extern TEST_CASE(test_timer);
+extern TEST_CASE(test_config);
 
 int main(int argc, const char *argv[])
 {
@@ -67,9 +68,13 @@ int main(int argc, const char *argv[])
         return EXIT_FAILURE;
     }
 
+    config.syslog = 0;
+    config.loglevel = ERROR;
+    config.bufsize = 16384;
+
     struct node_conf conf = {NULL, 0};
     struct context ctx;
-    context_init(&ctx, 0, ERROR);
+    context_init(&ctx);
     memcpy(&config.node, &conf, sizeof(config.node));
     slot_init_updater(&ctx);
 
@@ -82,6 +87,7 @@ int main(int argc, const char *argv[])
     RUN_CASE(test_socket);
     RUN_CASE(test_client);
     RUN_CASE(test_timer);
+    RUN_CASE(test_config);
 
     usleep(10000);
     slot_create_job(SLOT_UPDATER_QUIT);
diff --git a/tests/test.h b/tests/test.h
@@ -34,8 +34,7 @@
     do {                                                                \
         if (filter(#test_func, manager.test_func_filter)) {             \
             struct context ctx;                                         \
-            context_init(&ctx, 0, ERROR);                               \
-            config.bufsize = 16384;                                     \
+            context_init(&ctx);                                         \
             event_init(&ctx.loop, 1024);                                \
             enum test_result res = test_func(&ctx);                     \
             context_free(&ctx);                                         \
@@ -196,7 +195,7 @@ static void post_case()
 
 
 struct context;
-extern void context_init(struct context *ctx, bool syslog, int log_level);
+extern void context_init(struct context *ctx);
 extern void context_free(struct context *ctx);
 
 #endif
diff --git a/tests/test_config.c b/tests/test_config.c
@@ -0,0 +1,35 @@
+#include "test.h"
+
+extern int config_add(char *name, char *value);
+
+TEST(test_config_bind) {
+    char n[] = "bind";
+
+    ASSERT(config_add(n, "123456") == -1);
+    ASSERT(config_add(n, "123asf") == -1);
+    ASSERT(config_add(n, "-1243") == -1);
+    ASSERT(config_add(n, "") == -1);
+    ASSERT(config_add(n, "abc") == -1);
+    ASSERT(config_add(n, "2345") == 0);
+    ASSERT(config.bind == 2345);
+
+    PASS(NULL);
+}
+
+TEST(test_config_syslog) {
+    char n[] = "syslog";
+
+    ASSERT(config_add(n, "1") == 0);
+    ASSERT(config.syslog == 1);
+    ASSERT(config_add(n, "4") == 0);
+    ASSERT(config.syslog == 1);
+    ASSERT(config_add(n, "0") == 0);
+    ASSERT(config.syslog == 0);
+
+    PASS(NULL);
+}
+
+TEST_CASE(test_config) {
+    RUN_TEST(test_config_bind);
+    RUN_TEST(test_config_syslog);
+}
diff --git a/tests/test_socket.c b/tests/test_socket.c
@@ -12,6 +12,25 @@ TEST(test_socket_address_init) {
     PASS(NULL);
 }
 
+TEST(test_parse_port) {
+    uint16_t port;
+
+    ASSERT(socket_parse_port("abcd", &port) == -1);
+    ASSERT(socket_parse_port("123455", &port) == -1);
+    ASSERT(socket_parse_port("-234", &port) == -1);
+    ASSERT(socket_parse_port("12345", &port) == 0);
+    ASSERT(port == 12345);
+
+    ASSERT(socket_parse_port("", &port) == -1);
+
+    char *d6 = ":";
+    ASSERT(socket_parse_port(d6 + 1, &port) == -1);
+
+    ASSERT(socket_parse_port("12345a", &port) == -1);
+
+    PASS(NULL);
+}
+
 TEST(test_socket_parse_addr) {
     struct address address;
     memset(&address, 0, sizeof(address));
@@ -47,6 +66,7 @@ TEST(test_socket_parse_addr_wrong) {
 
 TEST_CASE(test_socket) {
     RUN_TEST(test_socket_address_init);
+    RUN_TEST(test_parse_port);
     RUN_TEST(test_socket_parse_addr);
     RUN_TEST(test_socket_parse_addr_wrong);
 }

Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@`
`21`	`21`
`22`	`22`	`static struct context *contexts;`
`23`	`23`
`24`		`-static void config_init()`
	`24`	`+void config_init()`
`25`	`25`	`{`
`26`	`26`	`memset(config.cluster, 0, CLUSTER_NAME_SIZE + 1);`
`27`	`27`	`strncpy(config.cluster, "default", CLUSTER_NAME_SIZE);`
`@@ -40,17 +40,18 @@ static void config_init()`
`40`	`40`	`config.metric_interval = 10;`
`41`	`41`	`}`
`42`	`42`
`43`		`-static int config_add(char name, char value)`
	`43`	`+int config_add(char name, char value)`
`44`	`44`	`{`
`45`	`45`	`int val;`
`46`	`46`	`if (strcmp(name, "cluster") == 0) {`
`47`	`47`	`if (strlen(value) <= 0) return 0;`
`48`	`48`	`strncpy(config.cluster, value, CLUSTER_NAME_SIZE);`
`49`	`49`	`} else if (strcmp(name, "bind") == 0) {`
`50`		`- config.bind = atoi(value);`
`51`		`- if (config.bind > 0xFFFF) return -1;`
	`50`	`+ if (socket_parse_port(value, &config.bind) == CORVUS_ERR) {`
	`51`	`+ return CORVUS_ERR;`
	`52`	`+ }`
`52`	`53`	`} else if (strcmp(name, "syslog") == 0) {`
`53`		`- config.syslog = atoi(value);`
	`54`	`+ config.syslog = atoi(value) ? 1 : 0;`
`54`	`55`	`} else if (strcmp(name, "thread") == 0) {`
`55`	`56`	`config.thread = atoi(value);`
`56`	`57`	`if (config.thread <= 0) config.thread = 4;`
`@@ -106,7 +107,7 @@ static int config_add(char name, char value)`
`106`	`107`	`return 0;`
`107`	`108`	`}`
`108`	`109`
`109`		`-static int read_conf(const char *filename)`
	`110`	`+int read_conf(const char *filename)`
`110`	`111`	`{`
`111`	`112`	`FILE *fp = fopen(filename, "r");`
`112`	`113`	`if (fp == NULL) {`
`@@ -137,7 +138,7 @@ static int read_conf(const char *filename)`
`137`	`138`	`return 0;`
`138`	`139`	`}`
`139`	`140`
`140`		`-static void quit()`
	`141`	`+void quit()`
`141`	`142`	`{`
`142`	`143`	`if (config.stats) stats_kill();`
`143`	`144`
`@@ -155,7 +156,7 @@ static void quit()`
`155`	`156`	`}`
`156`	`157`	`}`
`157`	`158`
`158`		`-static void log_traceback()`
	`159`	`+void log_traceback()`
`159`	`160`	`{`
`160`	`161`	`void *stack[64];`
`161`	`162`	`char **symbols;`
`@@ -185,7 +186,7 @@ static void log_traceback()`
`185`	`186`	`exit(EXIT_FAILURE);`
`186`	`187`	`}`
`187`	`188`
`188`		`-static void sig_handler(int sig)`
	`189`	`+void sig_handler(int sig)`
`189`	`190`	`{`
`190`	`191`	`switch (sig) {`
`191`	`192`	`case SIGINT:`
`@@ -198,7 +199,7 @@ static void sig_handler(int sig)`
`198`	`199`	`}`
`199`	`200`	`}`
`200`	`201`
`201`		`-static void setup_signal()`
	`202`	`+void setup_signal()`
`202`	`203`	`{`
`203`	`204`	`struct sigaction act;`
`204`	`205`	`sigemptyset(&act.sa_mask);`
`@@ -224,18 +225,15 @@ struct context *get_contexts()`
`224`	`225`	`return contexts;`
`225`	`226`	`}`
`226`	`227`
`227`		`-void context_init(struct context *ctx, bool syslog, int log_level)`
	`228`	`+void context_init(struct context *ctx)`
`228`	`229`	`{`
`229`	`230`	`memset(ctx, 0, sizeof(struct context));`
`230`	`231`
`231`		`- ctx->syslog = syslog;`
`232`		`- ctx->log_level = log_level;`
`233`	`232`	`dict_init(&ctx->server_table);`
`234`	`233`	`ctx->started = false;`
`235`	`234`	`ctx->role = THREAD_UNKNOWN;`
`236`	`235`	`ctx->state = CTX_UNKNOWN;`
`237`	`236`	`mbuf_init(ctx);`
`238`		`- log_init(ctx);`
`239`	`237`
`240`	`238`	`STAILQ_INIT(&ctx->free_cmdq);`
`241`	`239`	`TAILQ_INIT(&ctx->servers);`
`@@ -375,7 +373,7 @@ int main(int argc, const char *argv[])`
`375`	`373`
`376`	`374`	`contexts = malloc(sizeof(struct context) * (config.thread + 1));`
`377`	`375`	`for (i = 0; i <= config.thread; i++) {`
`378`		`- context_init(&contexts[i], config.syslog, config.loglevel);`
	`376`	`+ context_init(&contexts[i]);`
`379`	`377`	`contexts[i].role = THREAD_UNKNOWN;`
`380`	`378`	`}`
`381`	`379`