From b99037eae6a7af058cdb56f316d9c8e4ca603d86 Mon Sep 17 00:00:00 2001 From: Sander Vrijders Date: Thu, 1 Dec 2016 11:15:41 +0100 Subject: ipcpd: normal: Address review comments This addresses some comments after a code review by Dimitri. --- src/ipcpd/normal/path.c | 4 ++-- src/ipcpd/normal/path.h | 6 +++--- src/ipcpd/normal/pol/flat.c | 34 +++++++++++++++++++++------------- src/ipcpd/normal/ribmgr.c | 20 +++++++++----------- 4 files changed, 35 insertions(+), 29 deletions(-) (limited to 'src') diff --git a/src/ipcpd/normal/path.c b/src/ipcpd/normal/path.c index 08d33347..d9b64968 100644 --- a/src/ipcpd/normal/path.c +++ b/src/ipcpd/normal/path.c @@ -38,7 +38,7 @@ char * pathname_create(const char * name) assert(name); - tmp = malloc(strlen(name) + strlen(PATH_DELIMITER)); + tmp = malloc(strlen(name) + strlen(PATH_DELIMITER) + 1); if (tmp == NULL) return NULL; @@ -58,7 +58,7 @@ char * pathname_append(char * pname, tmp = malloc(strlen(pname) + strlen(PATH_DELIMITER) + - strlen(name)); + strlen(name) + 1); if (tmp == NULL) return NULL; diff --git a/src/ipcpd/normal/path.h b/src/ipcpd/normal/path.h index 022120d9..c43f3a26 100644 --- a/src/ipcpd/normal/path.h +++ b/src/ipcpd/normal/path.h @@ -20,8 +20,8 @@ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ -#ifndef OUROBOROS_PATHNAME -#define OUROBOROS_PATHNAME +#ifndef OUROBOROS_IPCPD_NORMAL_PATH_H +#define OUROBOROS_IPCPD_NORMAL_PATH_H #define PATH_DELIMITER "/" @@ -32,4 +32,4 @@ char * pathname_append(char * pname, void pathname_destroy(char * pname); -#endif +#endif /* OUROBOROS_IPCPD_NORMAL_PATH_H */ diff --git a/src/ipcpd/normal/pol/flat.c b/src/ipcpd/normal/pol/flat.c index bbfc85a4..a428da20 100644 --- a/src/ipcpd/normal/pol/flat.c +++ b/src/ipcpd/normal/pol/flat.c @@ -27,17 +27,17 @@ #include #include +#include "shm_pci.h" +#include "ribmgr.h" +#include "ro.h" +#include "path.h" + #include #include #include #include #include -#include "shm_pci.h" -#include "ribmgr.h" -#include "ro.h" -#include "path.h" - #define POL_RO_ROOT "flat_addr" #define TIMEOUT 100 /* ms */ @@ -63,7 +63,8 @@ struct { static char * addr_name(void) { char * name; - char addr_name[100]; + /* uint64_t as a string has 25 chars */ + char addr_name[30]; sprintf(addr_name, "%lu", (unsigned long) flat.addr); @@ -76,8 +77,8 @@ static char * addr_name(void) } static void ro_created(const char * name, - uint8_t * data, - size_t len) + uint8_t * data, + size_t len) { struct flat_addr_msg * msg; @@ -86,8 +87,7 @@ static void ro_created(const char * name, assert(len >= sizeof(*msg)); msg = (struct flat_addr_msg *) data; - if (msg->code == FLAT_ADDR_REQ && - msg->addr == flat.addr) { + if (msg->code == FLAT_ADDR_REQ && msg->addr == flat.addr) { msg->code = FLAT_ADDR_REPLY; ro_write(name, data, len); } @@ -146,8 +146,7 @@ int flat_init(void) #endif pthread_cond_init(&flat.cond, &cattr); - flat.sid = ro_subscribe(POL_RO_ROOT, - &flat_sub_ops); + flat.sid = ro_subscribe(POL_RO_ROOT, &flat_sub_ops); if (flat.sid < 0) { LOG_ERR("Could not subscribe to RIB."); pthread_cond_destroy(&flat.cond); @@ -171,6 +170,7 @@ int flat_init(void) ro_unsubscribe(flat.sid); return -1; } + pathname_destroy(name); return 0; @@ -201,6 +201,11 @@ uint64_t flat_address(void) if (dtc == NULL) return INVALID_ADDR; + if (dtc->addr_size == 8) { + LOG_ERR("Policy cannot be used with 64 bit addresses."); + return INVALID_ADDR; + } + while (ret != -ETIMEDOUT) { clock_gettime(PTHREAD_COND_CLOCK, &abstime); ts_add(&abstime, &timeout, &abstime); @@ -222,13 +227,16 @@ uint64_t flat_address(void) msg->addr = flat.addr; ro_name = addr_name(); - if (ro_name == NULL) + if (ro_name == NULL) { + free(buf); return INVALID_ADDR; + } pthread_mutex_lock(&flat.lock); if (ro_create(ro_name, &attr, buf, sizeof(*msg))) { pthread_mutex_unlock(&flat.lock); free(ro_name); + free(buf); return INVALID_ADDR; } free(ro_name); diff --git a/src/ipcpd/normal/ribmgr.c b/src/ipcpd/normal/ribmgr.c index 3cebde0c..3e305ffe 100644 --- a/src/ipcpd/normal/ribmgr.c +++ b/src/ipcpd/normal/ribmgr.c @@ -404,7 +404,7 @@ static struct rnode * ribmgr_ro_create(const char * name, timeout = attr.expiry.tv_sec * 1000 + attr.expiry.tv_nsec / MILLION; if (timerwheel_add(rib.wheel, ro_delete_timer, - new->full_name, strlen(new->full_name), + new->full_name, strlen(new->full_name) + 1, timeout)) { LOG_ERR("Failed to add deletion timer of RO."); } @@ -1400,8 +1400,7 @@ int ro_create(const char * name, ro_msg_t msg = RO_MSG__INIT; struct ro_attr rattr; - if (name == NULL) - return -EINVAL; + assert(name); if (attr == NULL) { ro_attr_init(&rattr); @@ -1456,8 +1455,7 @@ int ro_delete(const char * name) struct rnode * node; ro_msg_t msg = RO_MSG__INIT; - if (name == NULL) - return -EINVAL; + assert(name); pthread_mutex_lock(&rib.ro_lock); @@ -1499,8 +1497,8 @@ int ro_write(const char * name, struct rnode * node; ro_msg_t msg = RO_MSG__INIT; - if (name == NULL || data == NULL) - return -EINVAL; + assert(name); + assert(data); pthread_mutex_lock(&rib.ro_lock); @@ -1540,8 +1538,8 @@ ssize_t ro_read(const char * name, struct rnode * node; ssize_t len; - if (name == NULL || data == NULL) - return -EINVAL; + assert(name); + assert(data); pthread_mutex_lock(&rib.ro_lock); @@ -1570,8 +1568,8 @@ int ro_subscribe(const char * name, { struct ro_sub * sub; - if (name == NULL || ops == NULL) - return -EINVAL; + assert(name); + assert(ops); sub = malloc(sizeof(*sub)); if (sub == NULL) -- cgit v1.2.3