From 7a286471720aca037ea001f052630c16044198ee Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Tue, 27 Dec 2016 16:44:31 +0100 Subject: ipcpd: Use qoscube_t instead of enum qos_cube --- src/ipcpd/ipcp-ops.h | 8 ++++---- src/ipcpd/normal/fmgr.h | 12 ++++++------ src/ipcpd/normal/frct.c | 14 +++++++------- src/ipcpd/normal/frct.h | 12 ++++++------ 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/ipcpd/ipcp-ops.h b/src/ipcpd/ipcp-ops.h index 9872b00c..d780c6dc 100644 --- a/src/ipcpd/ipcp-ops.h +++ b/src/ipcpd/ipcp-ops.h @@ -37,10 +37,10 @@ struct ipcp_ops { int (* ipcp_name_query)(char * name); - int (* ipcp_flow_alloc)(int fd, - char * dst_ap_name, - char * src_ae_name, - enum qos_cube qos); + int (* ipcp_flow_alloc)(int fd, + char * dst_ap_name, + char * src_ae_name, + qoscube_t qos); int (* ipcp_flow_alloc_resp)(int fd, int response); diff --git a/src/ipcpd/normal/fmgr.h b/src/ipcpd/normal/fmgr.h index 30267666..e17f3f55 100644 --- a/src/ipcpd/normal/fmgr.h +++ b/src/ipcpd/normal/fmgr.h @@ -37,10 +37,10 @@ int fmgr_init(void); int fmgr_fini(void); -int fmgr_np1_alloc(int fd, - char * dst_ap_name, - char * src_ae_name, - enum qos_cube qos); +int fmgr_np1_alloc(int fd, + char * dst_ap_name, + char * src_ae_name, + qoscube_t qos); int fmgr_np1_alloc_resp(int fd, int response); @@ -55,8 +55,8 @@ int fmgr_np1_post_sdu(cep_id_t id, int fmgr_nm1_mgmt_flow(char * dst_name); -int fmgr_nm1_dt_flow(char * dst_name, - enum qos_cube qos); +int fmgr_nm1_dt_flow(char * dst_name, + qoscube_t qos); int fmgr_nm1_write_sdu(struct pci * pci, struct shm_du_buff * sdb); diff --git a/src/ipcpd/normal/frct.c b/src/ipcpd/normal/frct.c index a45e3528..33bd044b 100644 --- a/src/ipcpd/normal/frct.c +++ b/src/ipcpd/normal/frct.c @@ -43,7 +43,7 @@ struct frct_i { uint32_t cep_id; uint64_t r_address; uint32_t r_cep_id; - enum qos_cube cube; + qoscube_t cube; uint64_t seqno; enum conn_state state; @@ -217,9 +217,9 @@ static void destroy_frct_i(struct frct_i * instance) free(instance); } -cep_id_t frct_i_create(uint64_t address, - buffer_t * buf, - enum qos_cube cube) +cep_id_t frct_i_create(uint64_t address, + buffer_t * buf, + qoscube_t cube) { struct frct_i * instance; struct pci pci; @@ -255,9 +255,9 @@ cep_id_t frct_i_create(uint64_t address, return id; } -int frct_i_accept(cep_id_t id, - buffer_t * buf, - enum qos_cube cube) +int frct_i_accept(cep_id_t id, + buffer_t * buf, + qoscube_t cube) { struct pci pci; struct frct_i * instance; diff --git a/src/ipcpd/normal/frct.h b/src/ipcpd/normal/frct.h index 38dcddf4..39c47cdd 100644 --- a/src/ipcpd/normal/frct.h +++ b/src/ipcpd/normal/frct.h @@ -33,13 +33,13 @@ int frct_init(void); int frct_fini(void); -cep_id_t frct_i_create(uint64_t address, - buffer_t * buf, - enum qos_cube cube); +cep_id_t frct_i_create(uint64_t address, + buffer_t * buf, + qoscube_t cube); -int frct_i_accept(cep_id_t id, - buffer_t * buf, - enum qos_cube cube); +int frct_i_accept(cep_id_t id, + buffer_t * buf, + qoscube_t cube); int frct_i_destroy(cep_id_t id, buffer_t * buf); -- cgit v1.2.3 From 0324479318e76cecbe70b274cf2ab3479fe6f437 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Tue, 27 Dec 2016 19:18:06 +0100 Subject: ipcpd: Simplify ipcp_wait_state implementation --- src/ipcpd/ipcp.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/ipcpd/ipcp.c b/src/ipcpd/ipcp.c index 8bd2ba77..fdf3a340 100644 --- a/src/ipcpd/ipcp.c +++ b/src/ipcpd/ipcp.c @@ -143,6 +143,7 @@ int ipcp_wait_state(enum ipcp_state state, const struct timespec * timeout) { struct timespec abstime; + int ret = 0; clock_gettime(PTHREAD_COND_CLOCK, &abstime); ts_add(&abstime, timeout, &abstime); @@ -150,23 +151,18 @@ int ipcp_wait_state(enum ipcp_state state, pthread_mutex_lock(&ipcpi.state_mtx); while (ipcpi.state != state && ipcpi.state != IPCP_SHUTDOWN) { - int ret; if (timeout == NULL) - ret = pthread_cond_wait(&ipcpi.state_cond, - &ipcpi.state_mtx); + ret = -pthread_cond_wait(&ipcpi.state_cond, + &ipcpi.state_mtx); else - ret = pthread_cond_timedwait(&ipcpi.state_cond, - &ipcpi.state_mtx, - &abstime); - if (ret) { - pthread_mutex_unlock(&ipcpi.state_mtx); - return -ret; - } + ret = -pthread_cond_timedwait(&ipcpi.state_cond, + &ipcpi.state_mtx, + &abstime); } pthread_mutex_unlock(&ipcpi.state_mtx); - return 0; + return ret; } int ipcp_parse_arg(int argc, char * argv[]) -- cgit v1.2.3 From dbd901e65371ae390c36e7f6ed4a8bf59db6a5f6 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Tue, 27 Dec 2016 21:36:52 +0100 Subject: ipcpd: Hide the IPCP mainloop Since the mainloop threadpool is created on ipcp_init and all threads waited for on ipcp_fini, there is no need for it to be accessible from elsewhere. --- src/ipcpd/ipcp.c | 338 +++++++++++++++++++++++++++---------------------------- src/ipcpd/ipcp.h | 2 - 2 files changed, 169 insertions(+), 171 deletions(-) diff --git a/src/ipcpd/ipcp.c b/src/ipcpd/ipcp.c index fdf3a340..8c0bd0bf 100644 --- a/src/ipcpd/ipcp.c +++ b/src/ipcpd/ipcp.c @@ -19,7 +19,6 @@ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ - #define OUROBOROS_PREFIX "ipcpd/ipcp" #include @@ -37,174 +36,7 @@ #include #include -int ipcp_init(enum ipcp_type type, struct ipcp_ops * ops) -{ - pthread_condattr_t cattr; - int t; - - struct timeval tv = {(IPCP_ACCEPT_TIMEOUT / 1000), - (IPCP_ACCEPT_TIMEOUT % 1000) * 1000}; - - ipcpi.irmd_fd = -1; - ipcpi.state = IPCP_INIT; - - ipcpi.threadpool = malloc(sizeof(pthread_t) * IPCPD_THREADPOOL_SIZE); - if (ipcpi.threadpool == NULL) { - return -ENOMEM; - } - - ipcpi.sock_path = ipcp_sock_path(getpid()); - if (ipcpi.sock_path == NULL) { - free(ipcpi.threadpool); - return -1; - } - - ipcpi.sockfd = server_socket_open(ipcpi.sock_path); - if (ipcpi.sockfd < 0) { - LOG_ERR("Could not open server socket."); - free(ipcpi.threadpool); - free(ipcpi.sock_path); - return -1; - } - - if (setsockopt(ipcpi.sockfd, SOL_SOCKET, SO_RCVTIMEO, - (void *) &tv, sizeof(tv))) - LOG_WARN("Failed to set timeout on socket."); - - ipcpi.ops = ops; - - ipcpi.data = ipcp_data_create(); - if (ipcpi.data == NULL) { - free(ipcpi.threadpool); - free(ipcpi.sock_path); - return -ENOMEM; - } - - ipcp_data_init(ipcpi.data, type); - - pthread_rwlock_init(&ipcpi.state_lock, NULL); - pthread_mutex_init(&ipcpi.state_mtx, NULL); - pthread_condattr_init(&cattr); -#ifndef __APPLE__ - pthread_condattr_setclock(&cattr, PTHREAD_COND_CLOCK); -#endif - pthread_cond_init(&ipcpi.state_cond, &cattr); - - for (t = 0; t < IPCPD_THREADPOOL_SIZE; ++t) - pthread_create(&ipcpi.threadpool[t], NULL, - ipcp_main_loop, NULL); - - return 0; -} - -void ipcp_fini() -{ - int t; - - for (t = 0; t < IPCPD_THREADPOOL_SIZE; ++t) - pthread_join(ipcpi.threadpool[t], NULL); - - close(ipcpi.sockfd); - if (unlink(ipcpi.sock_path)) - LOG_DBG("Could not unlink %s.", ipcpi.sock_path); - - free(ipcpi.sock_path); - free(ipcpi.threadpool); - - ipcp_data_destroy(ipcpi.data); - pthread_cond_destroy(&ipcpi.state_cond); - pthread_rwlock_destroy(&ipcpi.state_lock); -} - -void ipcp_set_state(enum ipcp_state state) -{ - pthread_mutex_lock(&ipcpi.state_mtx); - - ipcpi.state = state; - - pthread_cond_broadcast(&ipcpi.state_cond); - pthread_mutex_unlock(&ipcpi.state_mtx); -} - -enum ipcp_state ipcp_get_state() -{ - enum ipcp_state state; - - pthread_mutex_lock(&ipcpi.state_mtx); - - state = ipcpi.state; - - pthread_mutex_unlock(&ipcpi.state_mtx); - - return state; -} - -int ipcp_wait_state(enum ipcp_state state, - const struct timespec * timeout) -{ - struct timespec abstime; - int ret = 0; - - clock_gettime(PTHREAD_COND_CLOCK, &abstime); - ts_add(&abstime, timeout, &abstime); - - pthread_mutex_lock(&ipcpi.state_mtx); - - while (ipcpi.state != state && ipcpi.state != IPCP_SHUTDOWN) { - if (timeout == NULL) - ret = -pthread_cond_wait(&ipcpi.state_cond, - &ipcpi.state_mtx); - else - ret = -pthread_cond_timedwait(&ipcpi.state_cond, - &ipcpi.state_mtx, - &abstime); - } - - pthread_mutex_unlock(&ipcpi.state_mtx); - - return ret; -} - -int ipcp_parse_arg(int argc, char * argv[]) -{ - char * log_file; - size_t len = 0; - - if (!(argc == 3 || argc == 2)) - return -1; - - /* argument 1: api of irmd */ - if (atoi(argv[1]) == 0) - return -1; - - if (argv[2] == NULL) - return 0; - - len += strlen(INSTALL_PREFIX); - len += strlen(LOG_DIR); - len += strlen(argv[2]); - - log_file = malloc(len + 1); - if (log_file == NULL) { - LOG_ERR("Failed to malloc"); - return -1; - } - - strcpy(log_file, INSTALL_PREFIX); - strcat(log_file, LOG_DIR); - strcat(log_file, argv[2]); - log_file[len] = '\0'; - - if (set_logfile(log_file)) - LOG_ERR("Cannot open %s, falling back to stdout for logs.", - log_file); - - free(log_file); - - return 0; -} - -void * ipcp_main_loop(void * o) +static void * ipcp_main_loop(void * o) { int lsockfd; uint8_t buf[IPCP_MSG_BUF_SIZE]; @@ -442,3 +274,171 @@ void * ipcp_main_loop(void * o) return (void *) 0; } + +int ipcp_init(enum ipcp_type type, struct ipcp_ops * ops) +{ + pthread_condattr_t cattr; + int t; + + struct timeval tv = {(IPCP_ACCEPT_TIMEOUT / 1000), + (IPCP_ACCEPT_TIMEOUT % 1000) * 1000}; + + ipcpi.irmd_fd = -1; + ipcpi.state = IPCP_INIT; + + ipcpi.threadpool = malloc(sizeof(pthread_t) * IPCPD_THREADPOOL_SIZE); + if (ipcpi.threadpool == NULL) { + return -ENOMEM; + } + + ipcpi.sock_path = ipcp_sock_path(getpid()); + if (ipcpi.sock_path == NULL) { + free(ipcpi.threadpool); + return -1; + } + + ipcpi.sockfd = server_socket_open(ipcpi.sock_path); + if (ipcpi.sockfd < 0) { + LOG_ERR("Could not open server socket."); + free(ipcpi.threadpool); + free(ipcpi.sock_path); + return -1; + } + + if (setsockopt(ipcpi.sockfd, SOL_SOCKET, SO_RCVTIMEO, + (void *) &tv, sizeof(tv))) + LOG_WARN("Failed to set timeout on socket."); + + ipcpi.ops = ops; + + ipcpi.data = ipcp_data_create(); + if (ipcpi.data == NULL) { + free(ipcpi.threadpool); + free(ipcpi.sock_path); + return -ENOMEM; + } + + ipcp_data_init(ipcpi.data, type); + + pthread_rwlock_init(&ipcpi.state_lock, NULL); + pthread_mutex_init(&ipcpi.state_mtx, NULL); + pthread_condattr_init(&cattr); +#ifndef __APPLE__ + pthread_condattr_setclock(&cattr, PTHREAD_COND_CLOCK); +#endif + pthread_cond_init(&ipcpi.state_cond, &cattr); + + for (t = 0; t < IPCPD_THREADPOOL_SIZE; ++t) + pthread_create(&ipcpi.threadpool[t], NULL, + ipcp_main_loop, NULL); + + return 0; +} + +void ipcp_fini() +{ + int t; + + for (t = 0; t < IPCPD_THREADPOOL_SIZE; ++t) + pthread_join(ipcpi.threadpool[t], NULL); + + close(ipcpi.sockfd); + if (unlink(ipcpi.sock_path)) + LOG_DBG("Could not unlink %s.", ipcpi.sock_path); + + free(ipcpi.sock_path); + free(ipcpi.threadpool); + + ipcp_data_destroy(ipcpi.data); + + pthread_cond_destroy(&ipcpi.state_cond); + pthread_rwlock_destroy(&ipcpi.state_lock); +} + +void ipcp_set_state(enum ipcp_state state) +{ + pthread_mutex_lock(&ipcpi.state_mtx); + + ipcpi.state = state; + + pthread_cond_broadcast(&ipcpi.state_cond); + pthread_mutex_unlock(&ipcpi.state_mtx); +} + +enum ipcp_state ipcp_get_state() +{ + enum ipcp_state state; + + pthread_mutex_lock(&ipcpi.state_mtx); + + state = ipcpi.state; + + pthread_mutex_unlock(&ipcpi.state_mtx); + + return state; +} + +int ipcp_wait_state(enum ipcp_state state, + const struct timespec * timeout) +{ + struct timespec abstime; + int ret = 0; + + clock_gettime(PTHREAD_COND_CLOCK, &abstime); + ts_add(&abstime, timeout, &abstime); + + pthread_mutex_lock(&ipcpi.state_mtx); + + while (ipcpi.state != state && ipcpi.state != IPCP_SHUTDOWN) { + if (timeout == NULL) + ret = -pthread_cond_wait(&ipcpi.state_cond, + &ipcpi.state_mtx); + else + ret = -pthread_cond_timedwait(&ipcpi.state_cond, + &ipcpi.state_mtx, + &abstime); + } + + pthread_mutex_unlock(&ipcpi.state_mtx); + + return ret; +} + +int ipcp_parse_arg(int argc, char * argv[]) +{ + char * log_file; + size_t len = 0; + + if (!(argc == 3 || argc == 2)) + return -1; + + /* argument 1: api of irmd */ + if (atoi(argv[1]) == 0) + return -1; + + if (argv[2] == NULL) + return 0; + + len += strlen(INSTALL_PREFIX); + len += strlen(LOG_DIR); + len += strlen(argv[2]); + + log_file = malloc(len + 1); + if (log_file == NULL) { + LOG_ERR("Failed to malloc"); + return -1; + } + + strcpy(log_file, INSTALL_PREFIX); + strcat(log_file, LOG_DIR); + strcat(log_file, argv[2]); + log_file[len] = '\0'; + + if (set_logfile(log_file)) + LOG_ERR("Cannot open %s, falling back to stdout for logs.", + log_file); + + free(log_file); + + return 0; +} diff --git a/src/ipcpd/ipcp.h b/src/ipcpd/ipcp.h index 6cd6f99e..f090f415 100644 --- a/src/ipcpd/ipcp.h +++ b/src/ipcpd/ipcp.h @@ -66,8 +66,6 @@ enum ipcp_state ipcp_get_state(void); int ipcp_wait_state(enum ipcp_state state, const struct timespec * timeout); -void * ipcp_main_loop(void * o); - int ipcp_parse_arg(int argc, char * argv[]); -- cgit v1.2.3 From 6166259f74855fe9e6976175eeb7b16912bc477a Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Tue, 27 Dec 2016 23:30:06 +0100 Subject: lib: Fix memleak in irm.c --- src/lib/irm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lib/irm.c b/src/lib/irm.c index 4b9a6285..0d9d0f86 100644 --- a/src/lib/irm.c +++ b/src/lib/irm.c @@ -358,6 +358,9 @@ int irm_bind_ap(char * ap, msg.opts = opts; recv_msg = send_recv_irm_msg(&msg); + + free(full_ap_name); + if (recv_msg == NULL) return -1; @@ -369,7 +372,6 @@ int irm_bind_ap(char * ap, ret = recv_msg->result; irm_msg__free_unpacked(recv_msg, NULL); - free(full_ap_name); return ret; } -- cgit v1.2.3 From 314c5323d3a038394f43e84d72594b78d17b1a46 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Wed, 28 Dec 2016 00:36:36 +0100 Subject: include, lib: Report communication failure with the IRMd An errno EIRMD has been added to errno.h to indicate failure to send a message tot the IRMd. The IRM tool will report such errors, which makes it easier for users to detect that the IRM was not started or has failed. --- include/ouroboros/errno.h | 1 + src/lib/dev.c | 42 +++++++++++------------------------------- src/lib/irm.c | 25 ++++++++++++------------- src/tools/irm/irm.c | 22 ++++++++++++++++++---- src/tools/irm/irm_bind_ap.c | 4 ++-- 5 files changed, 44 insertions(+), 50 deletions(-) diff --git a/include/ouroboros/errno.h b/include/ouroboros/errno.h index 0b55c00d..e50b180d 100644 --- a/include/ouroboros/errno.h +++ b/include/ouroboros/errno.h @@ -27,5 +27,6 @@ #define ENOTALLOC 1000 /* Flow is not allocated */ #define EIPCPTYPE 1001 /* Unknown IPCP type */ +#define EIRMD 1002 /* Failed to contact IRMD */ #endif diff --git a/src/lib/dev.c b/src/lib/dev.c index 8eb98506..3a1df16d 100644 --- a/src/lib/dev.c +++ b/src/lib/dev.c @@ -176,7 +176,7 @@ static int api_announce(char * ap_name) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) { - return -1; + return -EIRMD; } if (!recv_msg->has_result || (ret = recv_msg->result)) { @@ -359,7 +359,7 @@ int flow_accept(char ** ae_name, qosspec_t * qos) recv_msg = send_recv_irm_msg_b(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (!recv_msg->has_api || !recv_msg->has_port_id) { irm_msg__free_unpacked(recv_msg, NULL); @@ -465,7 +465,7 @@ int flow_alloc_resp(int fd, int response) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (!recv_msg->has_result) { irm_msg__free_unpacked(recv_msg, NULL); @@ -476,16 +476,6 @@ int flow_alloc_resp(int fd, int response) irm_msg__free_unpacked(recv_msg, NULL); - if (response) { - pthread_rwlock_rdlock(&ai.data_lock); - pthread_rwlock_wrlock(&ai.flows_lock); - - reset_flow(fd); - - pthread_rwlock_unlock(&ai.flows_lock); - pthread_rwlock_unlock(&ai.data_lock); - } - return ret; } @@ -517,7 +507,7 @@ int flow_alloc(char * dst_name, char * src_ae_name, qosspec_t * qos) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (!recv_msg->has_api || !recv_msg->has_port_id) { irm_msg__free_unpacked(recv_msg, NULL); @@ -603,27 +593,17 @@ int flow_alloc_res(int fd) recv_msg = send_recv_irm_msg_b(&msg); if (recv_msg == NULL) - result = -1; + return -EIRMD; if (!recv_msg->has_result) { irm_msg__free_unpacked(recv_msg, NULL); - result = -1; + return -1; } result = recv_msg->result; irm_msg__free_unpacked(recv_msg, NULL); - if (result) { - pthread_rwlock_rdlock(&ai.data_lock); - pthread_rwlock_wrlock(&ai.flows_lock); - - reset_flow(fd); - - pthread_rwlock_unlock(&ai.flows_lock); - pthread_rwlock_unlock(&ai.data_lock); - } - return result; } @@ -655,9 +635,9 @@ int flow_dealloc(int fd) pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); - recv_msg = send_recv_irm_msg_b(&msg); + recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (!recv_msg->has_result) { irm_msg__free_unpacked(recv_msg, NULL); @@ -1208,7 +1188,7 @@ int ipcp_create_r(pid_t api) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -1254,7 +1234,7 @@ int ipcp_flow_req_arr(pid_t api, char * dst_name, char * src_ae_name) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (!recv_msg->has_port_id || !recv_msg->has_api) { irm_msg__free_unpacked(recv_msg, NULL); @@ -1336,7 +1316,7 @@ int ipcp_flow_alloc_reply(int fd, int response) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); diff --git a/src/lib/irm.c b/src/lib/irm.c index 0d9d0f86..969a78c3 100644 --- a/src/lib/irm.c +++ b/src/lib/irm.c @@ -47,7 +47,7 @@ pid_t irm_create_ipcp(char * name, recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -75,7 +75,7 @@ int irm_destroy_ipcp(pid_t api) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -147,9 +147,8 @@ int irm_bootstrap_ipcp(pid_t api, } recv_msg = send_recv_irm_msg(&msg); - if (recv_msg == NULL) { - return -1; - } + if (recv_msg == NULL) + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -179,7 +178,7 @@ ssize_t irm_list_ipcps(char * name, recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) { free(msg.dif_name); - return -1; + return -EIRMD; } if (recv_msg->apis == NULL) { @@ -227,7 +226,7 @@ int irm_enroll_ipcp(pid_t api, recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) { free(msg.dif_name); - return -1; + return -EIRMD; } if (recv_msg->has_result == false) { @@ -362,7 +361,7 @@ int irm_bind_ap(char * ap, free(full_ap_name); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -391,7 +390,7 @@ int irm_bind_api(pid_t api, char * name) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -419,7 +418,7 @@ int irm_unbind_ap(char * ap, char * name) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -448,7 +447,7 @@ int irm_unbind_api(pid_t api, char * name) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -481,7 +480,7 @@ int irm_reg(char * name, recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -515,7 +514,7 @@ int irm_unreg(char * name, recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); diff --git a/src/tools/irm/irm.c b/src/tools/irm/irm.c index 7b2f3331..ce92d3d5 100644 --- a/src/tools/irm/irm.c +++ b/src/tools/irm/irm.c @@ -20,12 +20,15 @@ */ #include -#include -#include +#include #include "irm_ops.h" #include "irm_utils.h" +#include +#include +#include + static void usage(void) { printf("Usage: irm [OPERATION]\n\n" @@ -71,12 +74,23 @@ static int do_cmd(const char * argv0, return -1; } -int main(int argc, char ** argv) { +int main(int argc, char ** argv) +{ + int ret = 0; if (argc < 2) { usage(); return -1; } - return do_cmd(argv[1], argc - 1, argv + 1); + ret = do_cmd(argv[1], argc - 1, argv + 1); + + if (ret == -EIRMD) + printf("Failed to communicate with the " + "Ouroboros IPC Resource Manager daemon.\n"); + + if (ret) + exit(EXIT_FAILURE); + + exit(EXIT_SUCCESS); } diff --git a/src/tools/irm/irm_bind_ap.c b/src/tools/irm/irm_bind_ap.c index e8e5eb26..a5be5a03 100644 --- a/src/tools/irm/irm_bind_ap.c +++ b/src/tools/irm/irm_bind_ap.c @@ -90,13 +90,13 @@ int do_bind_ap(int argc, char ** argv) ret = irm_bind_ap(ap_name, name, flags, argc, argv); if (ret == -ENOENT) { printf("%s does not exist.\n", ap_name); - return -1; + return ret; } if (ret == -EPERM) { printf("Cannot execute %s, please check permissions.\n", ap_name); - return -1; + return ret; } if (temp != NULL) -- cgit v1.2.3 From 6d14473d4fd0629125d9a96d9292deb32ba7e0a8 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Wed, 28 Dec 2016 00:41:31 +0100 Subject: lib: Fix leaks in cdap Fixes a memleak of cdap_req structs and correcly releases invoke_ids if the cdap_request_send operation fails during its execution. --- src/lib/cdap.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/lib/cdap.c b/src/lib/cdap.c index 5671c515..17104770 100644 --- a/src/lib/cdap.c +++ b/src/lib/cdap.c @@ -105,7 +105,8 @@ static int release_invoke_id(struct cdap * instance, int id) #define cdap_sent_has_key(i, key) (cdap_sent_get_by_key(i, key) != NULL) -struct cdap_req * cdap_sent_get_by_key(struct cdap * instance, cdap_key_t key) +static struct cdap_req * cdap_sent_get_by_key(struct cdap * instance, + cdap_key_t key) { struct list_head * p = NULL; struct cdap_req * req = NULL; @@ -128,13 +129,17 @@ struct cdap_req * cdap_sent_get_by_key(struct cdap * instance, cdap_key_t key) return NULL; } -static int cdap_sent_add(struct cdap * instance, struct cdap_req * req) +static struct cdap_req * cdap_sent_add(struct cdap * instance, cdap_key_t key) { - assert (instance); - assert (req); + struct cdap_req * req; - if (cdap_sent_has_key(instance, req->key)) - return -EPERM; + assert(instance); + assert(key >= 0); + assert(!cdap_sent_has_key(instance, key)); + + req = cdap_req_create(key); + if (req == NULL) + return NULL; pthread_rwlock_wrlock(&instance->sent_lock); @@ -142,7 +147,7 @@ static int cdap_sent_add(struct cdap * instance, struct cdap_req * req) pthread_rwlock_unlock(&instance->sent_lock); - return 0; + return req; } static void cdap_sent_del(struct cdap * instance, struct cdap_req * req) @@ -157,6 +162,8 @@ static void cdap_sent_del(struct cdap * instance, struct cdap_req * req) list_del(&req->next); pthread_rwlock_unlock(&instance->sent_lock); + + cdap_req_destroy(req); } static void cdap_sent_destroy(struct cdap * instance) @@ -249,6 +256,7 @@ static void * sdu_reader(void * o) free(rcvd); continue; } + rcvd->iid = msg->invoke_id; rcvd->flags = msg->flags; rcvd->name = strdup(msg->name); @@ -494,18 +502,15 @@ cdap_key_t cdap_request_send(struct cdap * instance, key = invoke_id_to_key(iid); - req = cdap_req_create(key); - if (req == NULL) - return INVALID_CDAP_KEY; - - if (cdap_sent_add(instance, req)) { - cdap_req_destroy(req); + req = cdap_sent_add(instance, key); + if (req == NULL) { + release_invoke_id(instance, iid); return INVALID_CDAP_KEY; } if (write_msg(instance, &msg)) { cdap_sent_del(instance, req); - cdap_req_destroy(req); + release_invoke_id(instance, iid); return INVALID_CDAP_KEY; } -- cgit v1.2.3 From 502e7bb39096f6ce7718c29d9616bbd0314d045e Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Wed, 28 Dec 2016 09:05:44 +0100 Subject: lib: Fix race in destruction of cdap_req If cdap_req_destroy was called while in REQ_DONE, cdap->state would be accessed in cdap_req_respond. --- src/lib/cdap_req.c | 25 ++++++++++++++++--------- src/lib/cdap_req.h | 3 ++- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/lib/cdap_req.c b/src/lib/cdap_req.c index f80b10d7..57ad22c5 100644 --- a/src/lib/cdap_req.c +++ b/src/lib/cdap_req.c @@ -70,14 +70,14 @@ void cdap_req_destroy(struct cdap_req * creq) } if (creq->state == REQ_INIT) - creq->state = REQ_DONE; + creq->state = REQ_NULL; if (creq->state == REQ_PENDING) { creq->state = REQ_DESTROY; pthread_cond_broadcast(&creq->cond); } - while (creq->state != REQ_DONE) + while (creq->state != REQ_NULL) pthread_cond_wait(&creq->cond, &creq->lock); pthread_mutex_unlock(&creq->lock); @@ -109,17 +109,21 @@ int cdap_req_wait(struct cdap_req * creq) creq->state = REQ_PENDING; while (creq->state == REQ_PENDING) { - if ((ret = -pthread_cond_timedwait(&creq->cond, - &creq->lock, - &abstime)) == -ETIMEDOUT) + ret = -pthread_cond_timedwait(&creq->cond, + &creq->lock, + &abstime); + if (ret == -ETIMEDOUT) break; } - if (creq->state == REQ_DESTROY) + if (creq->state == REQ_DESTROY) { ret = -1; - - creq->state = REQ_DONE; - pthread_cond_broadcast(&creq->cond); + creq->state = REQ_NULL; + pthread_cond_broadcast(&creq->cond); + } else { + creq->state = REQ_DONE; + pthread_cond_broadcast(&creq->cond); + } pthread_mutex_unlock(&creq->lock); @@ -146,5 +150,8 @@ void cdap_req_respond(struct cdap_req * creq, int response, buffer_t data) while (creq->state == REQ_RESPONSE) pthread_cond_wait(&creq->cond, &creq->lock); + creq->state = REQ_NULL; + pthread_cond_broadcast(&creq->cond); + pthread_mutex_unlock(&creq->lock); } diff --git a/src/lib/cdap_req.h b/src/lib/cdap_req.h index 9d5cb0c8..b2ded060 100644 --- a/src/lib/cdap_req.h +++ b/src/lib/cdap_req.h @@ -31,7 +31,8 @@ #include enum creq_state { - REQ_INIT = 0, + REQ_NULL = 0, + REQ_INIT, REQ_PENDING, REQ_RESPONSE, REQ_DONE, -- cgit v1.2.3 From f8e230049143412ac424d4cbfd4ca95b6c7f64e1 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Wed, 28 Dec 2016 09:51:08 +0100 Subject: irmd: Hide reg_entry internal lock --- src/irmd/main.c | 70 +++++++++-------------------------------------------- src/irmd/registry.c | 51 ++++++++++++++++++++++++++++++++++++++ src/irmd/registry.h | 60 +++++++++++++++++++++++++-------------------- 3 files changed, 97 insertions(+), 84 deletions(-) diff --git a/src/irmd/main.c b/src/irmd/main.c index 5ba61fa0..c42f2d3c 100644 --- a/src/irmd/main.c +++ b/src/irmd/main.c @@ -973,11 +973,8 @@ static struct irm_flow * flow_accept(pid_t api, char ** dst_ae_name) struct str_el * s = list_entry(p, struct str_el, next); LOG_DBG(" %s", s->str); re = registry_get_entry(&irmd->registry, s->str); - if (re != NULL) { - pthread_mutex_lock(&re->state_lock); + if (re != NULL) reg_entry_add_api(re, api); - pthread_mutex_unlock(&re->state_lock); - } } pthread_rwlock_unlock(&irmd->reg_lock); @@ -1015,17 +1012,12 @@ static struct irm_flow * flow_accept(pid_t api, char ** dst_ae_name) pthread_mutex_unlock(&e->state_lock); - pthread_mutex_lock(&re->state_lock); - - if (re->state != REG_NAME_FLOW_ARRIVED) { - pthread_mutex_unlock(&re->state_lock); + if (reg_entry_get_state(re) != REG_NAME_FLOW_ARRIVED) { pthread_rwlock_unlock(&irmd->reg_lock); pthread_rwlock_unlock(&irmd->state_lock); return NULL; } - pthread_mutex_unlock(&re->state_lock); pthread_rwlock_unlock(&irmd->reg_lock); - pthread_rwlock_rdlock(&irmd->flows_lock); f = get_irm_flow_n(api); @@ -1085,22 +1077,16 @@ static int flow_alloc_resp(pid_t n_api, return -1; } - pthread_mutex_lock(&re->state_lock); - - if (re->state != REG_NAME_FLOW_ARRIVED) { - pthread_mutex_unlock(&re->state_lock); + if (reg_entry_get_state(re) != REG_NAME_FLOW_ARRIVED) { pthread_rwlock_unlock(&irmd->reg_lock); pthread_rwlock_unlock(&irmd->state_lock); LOG_ERR("Name %s has no pending flow request.", re->name); return -1; } - pthread_mutex_unlock(&re->state_lock); - registry_del_api(&irmd->registry, n_api); pthread_rwlock_unlock(&irmd->reg_lock); - pthread_rwlock_wrlock(&irmd->flows_lock); f = get_irm_flow(port_id); @@ -1335,8 +1321,6 @@ static struct irm_flow * flow_req_arr(pid_t api, struct api_entry * e = NULL; struct irm_flow * f = NULL; - enum reg_name_state state; - struct pid_el * c_api; pid_t h_api = -1; int port_id = -1; @@ -1355,11 +1339,7 @@ static struct irm_flow * flow_req_arr(pid_t api, return NULL; } - pthread_mutex_lock(&re->state_lock); - state = re->state; - pthread_mutex_unlock(&re->state_lock); - - switch (state) { + switch (reg_entry_get_state(re)) { case REG_NAME_IDLE: pthread_rwlock_unlock(&irmd->reg_lock); pthread_rwlock_unlock(&irmd->state_lock); @@ -1373,16 +1353,12 @@ static struct irm_flow * flow_req_arr(pid_t api, return NULL; } - pthread_mutex_lock(&re->state_lock); - - re->state = REG_NAME_AUTO_EXEC; + reg_entry_set_state(re, REG_NAME_AUTO_EXEC); a = apn_table_get_by_apn(&irmd->apn_table, reg_entry_get_apn(re)); - pthread_mutex_unlock(&re->state_lock); + if (a == NULL || (c_api->pid = auto_execute(a->argv)) < 0) { - pthread_mutex_lock(&re->state_lock); - re->state = REG_NAME_AUTO_ACCEPT; - pthread_mutex_unlock(&re->state_lock); + reg_entry_set_state(re, REG_NAME_AUTO_ACCEPT); pthread_rwlock_unlock(&irmd->reg_lock); pthread_rwlock_unlock(&irmd->state_lock); LOG_ERR("Could not get start apn for reg_entry %s.", @@ -1396,32 +1372,19 @@ static struct irm_flow * flow_req_arr(pid_t api, pthread_rwlock_unlock(&irmd->reg_lock); pthread_rwlock_unlock(&irmd->state_lock); - pthread_mutex_lock(&re->state_lock); - - while (re->state == REG_NAME_AUTO_EXEC) - pthread_cond_wait(&re->state_cond, &re->state_lock); - - pthread_mutex_unlock(&re->state_lock); + reg_entry_leave_state(re, REG_NAME_AUTO_EXEC); pthread_rwlock_rdlock(&irmd->state_lock); pthread_rwlock_rdlock(&irmd->reg_lock); - pthread_mutex_lock(&re->state_lock); - - if (re->state == REG_NAME_DESTROY) { - re->state = REG_NAME_NULL; - pthread_mutex_unlock(&re->state_lock); + if (reg_entry_get_state(re) == REG_NAME_DESTROY) { + reg_entry_set_state(re, REG_NAME_NULL); pthread_rwlock_unlock(&irmd->reg_lock); pthread_rwlock_unlock(&irmd->state_lock); return NULL; } - - pthread_mutex_unlock(&re->state_lock); - case REG_NAME_FLOW_ACCEPT: - pthread_mutex_lock(&re->state_lock); h_api = reg_entry_get_api(re); - pthread_mutex_unlock(&re->state_lock); if (h_api == -1) { pthread_rwlock_unlock(&irmd->reg_lock); pthread_rwlock_unlock(&irmd->state_lock); @@ -1459,14 +1422,10 @@ static struct irm_flow * flow_req_arr(pid_t api, list_add(&f->next, &irmd->irm_flows); pthread_rwlock_unlock(&irmd->flows_lock); - pthread_rwlock_rdlock(&irmd->reg_lock); - pthread_mutex_lock(&re->state_lock); re->req_ae_name = ae_name; - re->state = REG_NAME_FLOW_ARRIVED; - - pthread_mutex_unlock(&re->state_lock); + reg_entry_set_state(re, REG_NAME_FLOW_ARRIVED); e = api_table_get(&irmd->api_table, h_api); if (e == NULL) { @@ -1487,12 +1446,7 @@ static struct irm_flow * flow_req_arr(pid_t api, pthread_rwlock_unlock(&irmd->reg_lock); pthread_rwlock_unlock(&irmd->state_lock); - pthread_mutex_lock(&re->state_lock); - - while (re->state == REG_NAME_FLOW_ARRIVED) - pthread_cond_wait(&re->state_cond, &re->state_lock); - - pthread_mutex_unlock(&re->state_lock); + reg_entry_leave_state(re, REG_NAME_FLOW_ARRIVED); return f; } diff --git a/src/irmd/registry.c b/src/irmd/registry.c index 61d562fc..34d0a921 100644 --- a/src/irmd/registry.c +++ b/src/irmd/registry.c @@ -278,6 +278,9 @@ int reg_entry_add_api(struct reg_entry * e, pid_t api) return -ENOMEM; i->pid = api; + + pthread_mutex_lock(&e->state_lock); + list_add(&i->next, &e->reg_apis); if (e->state == REG_NAME_IDLE || @@ -287,6 +290,8 @@ int reg_entry_add_api(struct reg_entry * e, pid_t api) pthread_cond_signal(&e->state_cond); } + pthread_mutex_unlock(&e->state_lock); + return 0; } @@ -329,6 +334,52 @@ pid_t reg_entry_get_api(struct reg_entry * e) return list_first_entry(&e->reg_apis, struct pid_el, next)->pid; } +enum reg_name_state reg_entry_get_state(struct reg_entry * e) +{ + enum reg_name_state state; + + if (e == NULL) + return REG_NAME_NULL; + + pthread_mutex_lock(&e->state_lock); + + state = e->state; + + pthread_mutex_unlock(&e->state_lock); + + return state; +} + +int reg_entry_set_state(struct reg_entry * e, enum reg_name_state state) +{ + if (state == REG_NAME_DESTROY) + return -EPERM; + + pthread_mutex_lock(&e->state_lock); + + e->state = state; + pthread_cond_broadcast(&e->state_cond); + + pthread_mutex_unlock(&e->state_lock); + + return 0; +} + +int reg_entry_leave_state(struct reg_entry * e, enum reg_name_state state) +{ + if (e == NULL || state == REG_NAME_DESTROY) + return -EINVAL; + + pthread_mutex_lock(&e->state_lock); + + while (e->state == state) + pthread_cond_wait(&e->state_cond, &e->state_lock); + + pthread_mutex_unlock(&e->state_lock); + + return 0; +} + struct reg_entry * registry_get_entry(struct list_head * registry, char * name) { diff --git a/src/irmd/registry.h b/src/irmd/registry.h index f0da9267..1cb1d921 100644 --- a/src/irmd/registry.h +++ b/src/irmd/registry.h @@ -67,46 +67,54 @@ struct reg_entry { pthread_mutex_t state_lock; }; -int reg_entry_add_apn(struct reg_entry * e, - struct apn_entry * a); +int reg_entry_add_apn(struct reg_entry * e, + struct apn_entry * a); -void reg_entry_del_apn(struct reg_entry * e, - char * apn); +void reg_entry_del_apn(struct reg_entry * e, + char * apn); -char * reg_entry_get_apn(struct reg_entry * e); +char * reg_entry_get_apn(struct reg_entry * e); -int reg_entry_add_api(struct reg_entry * e, - pid_t api); +int reg_entry_add_api(struct reg_entry * e, + pid_t api); -void reg_entry_del_api(struct reg_entry * e, - pid_t api); +void reg_entry_del_api(struct reg_entry * e, + pid_t api); -pid_t reg_entry_get_api(struct reg_entry * e); +pid_t reg_entry_get_api(struct reg_entry * e); -struct reg_entry * registry_add_name(struct list_head * registry, - char * name); +enum reg_name_state reg_entry_get_state(struct reg_entry * e); -void registry_del_name(struct list_head * registry, - char * name); +int reg_entry_set_state(struct reg_entry * e, + enum reg_name_state state); -void registry_del_api(struct list_head * registry, - pid_t api); +int reg_entry_leave_state(struct reg_entry * e, + enum reg_name_state state); -void registry_sanitize_apis(struct list_head * registry); +struct reg_entry * registry_add_name(struct list_head * registry, + char * name); -struct reg_entry * registry_get_entry(struct list_head * registry, +void registry_del_name(struct list_head * registry, char * name); -int registry_add_name_to_dif(struct list_head * registry, - char * name, - char * dif_name, - enum ipcp_type type); +void registry_del_api(struct list_head * registry, + pid_t api); + +void registry_sanitize_apis(struct list_head * registry); + +struct reg_entry * registry_get_entry(struct list_head * registry, + char * name); + +int registry_add_name_to_dif(struct list_head * registry, + char * name, + char * dif_name, + enum ipcp_type type); -void registry_del_name_from_dif(struct list_head * registry, - char * name, - char * dif_name); +void registry_del_name_from_dif(struct list_head * registry, + char * name, + char * dif_name); -void registry_destroy(struct list_head * registry); +void registry_destroy(struct list_head * registry); #endif /* OUROBOROS_IRMD_REGISTRY_H */ -- cgit v1.2.3