logger: use callbacks in case of entity type enum 91/129491/8
authorRafal Pietruch <r.pietruch@samsung.com>
Tue, 16 May 2017 13:58:16 +0000 (15:58 +0200)
committerRafal Pietruch <r.pietruch@samsung.com>
Thu, 18 May 2017 07:49:15 +0000 (09:49 +0200)
Change-Id: Ibbc0447ebcb57e3d4a05c0ba09a8fe41ea2bd747

include/logpipe.h
src/logger/logger.c

index a912eb9..4fe63d5 100644 (file)
@@ -37,7 +37,8 @@ enum {
 
 struct dlog_control_msg {
        unsigned char length;
-       char request;
+       signed char request;    /* signed - declared explicitly to suppress static analyzer SVACE warning: */
+                                                       /* "In some build environments it [char] can be interpreted as 'unsigned char' [...]" */
        char flags;
        char data[0];
 };
index bfbef95..f965298 100644 (file)
@@ -74,18 +74,19 @@ enum reader_type {
        READER_LOGGER
 };
 
-enum entity_type {
-       ENTITY_WRITER = 1,
-       ENTITY_SOCK_CONTROL,
-       ENTITY_SOCK_WRITE,
-};
+struct logger;
 
 struct fd_entity {
-       int type;
+       /**
+        * @brief Event handler
+        * @details Receives and handles an event
+        * @param[in] server The logger server
+        * @param[in] event The received event
+        */
+       void (*dispatch_event)(struct logger *server, struct epoll_event *event);
 };
 
 struct writer;
-struct logger;
 
 /**
  * @brief Service a writer
@@ -97,16 +98,24 @@ struct logger;
  */
 typedef int (*service_writer_t)(struct logger* server, struct writer* wr, struct epoll_event* event);
 
+/**
+ * @brief Service socket request
+ * @details Handle request on socket in respect to msg request type
+ * @param[in] server The logger server
+ * @param[in] wr The writer to handle the request on the socket
+ * @param[in] msg The message containing the request
+ * @return 0 on success, else -errno
+ */
+typedef int (*service_socket_t)(struct logger *server, struct writer *wr, struct dlog_control_msg *msg);
+
 struct writer {
        struct fd_entity   _entity;
        int                working_fd;
        struct epoll_event event;
-       int                rights;
-
        struct log_buffer* buf_ptr;
-
        int                readed;
        service_writer_t   service_writer;
+       service_socket_t   service_socket;
        char               buffer[LOG_MAX_SIZE];
 };
 
@@ -128,6 +137,7 @@ struct sock_data {
        int                fd;
        struct log_buffer* buf_ptr;
        struct epoll_event event;
+       service_socket_t   service_socket;
 };
 
 struct log_buffer {
@@ -189,10 +199,14 @@ static int service_writer_socket(struct logger* server, struct writer* wr, struc
 static int service_writer_pipe(struct logger* server, struct writer* wr, struct epoll_event* event);
 static int service_writer_kmsg(struct logger* server, struct writer* wr, struct epoll_event* event);
 static int service_writer_syslog(struct logger* server, struct writer* wr, struct epoll_event* event);
+static int service_writer_handle_req_ctrl(struct logger *server, struct writer *wr, struct dlog_control_msg *msg);
+static int service_writer_handle_req_pipe(struct logger *server, struct writer *wr, struct dlog_control_msg *msg);
+static void dispatch_event_writer(struct logger *server, struct epoll_event *event);
+static void dispatch_event_sock(struct logger *server, struct epoll_event *event);
 static void reader_free(struct reader* reader);
 static void logger_free(struct logger* l);
 static int parse_args(int argc, char ** argv, struct buffering * b);
-static int socket_initialize(struct sock_data * sock, struct log_buffer * buffer, int type, struct socket_config_data * data, int epoll_fd);
+static int socket_initialize(struct sock_data *sock, struct log_buffer *buffer, service_socket_t service_socket, struct socket_config_data *data, int epoll_fd);
 static void fd_set_flags(int fd, int mask);
 static void fd_clear_flags(int fd, int mask);
 static void help();
@@ -420,35 +434,51 @@ static inline int remove_fd_loop(int epollfd, int fd)
  * @details Create a writer structure
  * @param[out] writer The newly-allocated writer
  * @param[in] fd The FD writer initially belongs to
- * @param[in] rights Whether the writer is connected to a control socket or not (used only for pipes)
  * @param[in] log_buffer The logging buffer
  * @param[in] service_writer callback function for handling new data on writer descriptor FD
  * @return 0 on success, -ENOMEM when out of memory
  */
-static int writer_create(struct writer **writer, int fd, int rights,
-                                                struct log_buffer *log_buffer, service_writer_t service_writer)
+static int writer_create(struct writer **writer, int fd, struct log_buffer *log_buffer,
+                                                service_writer_t service_writer, service_socket_t service_socket)
 {
        struct writer* w = calloc(1, sizeof(struct writer));
        if (!w)
                return -ENOMEM;
 
        *w = (struct writer) {
-               ._entity = (struct fd_entity) { .type = ENTITY_WRITER },
+               ._entity = (struct fd_entity) { .dispatch_event = dispatch_event_writer },
                .working_fd = fd,
                .event = (struct epoll_event) {
                        .data = (epoll_data_t) { .ptr = w },
                        .events = EPOLLIN
                },
-               .rights = rights,
                .buf_ptr = log_buffer,
                .readed = 0,
-               .service_writer = service_writer
+               .service_writer = service_writer,
+               .service_socket = service_socket
        };
        *writer = w;
        return 0;
 }
 
 /**
+ * @brief Dummy function placeholder that should not be called at all
+ * @details Use this function as a service socket callback for writers that handle other resources than sockets
+ *          This function is temporary and only for testing purposes
+ * @param[in] server Unused
+ * @param[in] wr Unused
+ * @param[in] msg Unused
+ * @return Will not return - just raises SIGABRT signal
+ *
+ * @todo Remove this function after unification of evant handling functionality of writers
+ */
+static int service_socket_dummy(struct logger *server, struct writer *wr, struct dlog_control_msg *msg)
+{
+       assert(0 && "This function should not be called");
+       return -1;
+}
+
+/**
  * @brief Create kmsg writer
  * @details Create the structure responsible for handling writing data from /dev/kmsg to given logging buffer
  * @param[out] writer The newly-allocated writer
@@ -460,7 +490,7 @@ static int create_kmsg_writer(struct writer ** writer, struct log_buffer *log_bu
        int fd = open("/dev/kmsg", O_RDONLY);
        if (fd < 0)
                return -errno;
-       return writer_create(writer, fd, 0, log_buffer, service_writer_kmsg);
+       return writer_create(writer, fd, log_buffer, service_writer_kmsg, service_socket_dummy);
 }
 
 static int systemd_sock_get(const char *path, int type)
@@ -513,7 +543,7 @@ static int create_syslog_writer(struct writer ** writer, struct log_buffer *log_
        if (fd < 0)
                return -errno;
        fd_set_flags(fd, O_NONBLOCK);
-       return writer_create(writer, fd, 0, log_buffer, service_writer_syslog);
+       return writer_create(writer, fd, log_buffer, service_writer_syslog, service_socket_dummy);
 }
 
 /**
@@ -549,10 +579,10 @@ static int buffer_create(struct log_buffer ** lb, int buf_id, struct buffer_conf
        (*lb)->sock_wr.fd = -1;
 
        int r;
-       r = socket_initialize(&(*lb)->sock_ctl, *lb, ENTITY_SOCK_CONTROL, &data->ctl_socket, epoll_fd);
+       r = socket_initialize(&(*lb)->sock_ctl, *lb, service_writer_handle_req_ctrl, &data->ctl_socket, epoll_fd);
        if (r < 0)
                return r;
-       r = socket_initialize(&(*lb)->sock_wr, *lb, ENTITY_SOCK_WRITE, &data->write_socket, epoll_fd);
+       r = socket_initialize(&(*lb)->sock_wr, *lb, service_writer_handle_req_pipe, &data->write_socket, epoll_fd);
        if (r < 0)
                return r;
 
@@ -1248,6 +1278,15 @@ static void fd_clear_flags(int fd, int mask)
  */
 static int service_writer_handle_req_util(struct logger* server, struct writer* wr, struct dlog_control_msg* msg)
 {
+       assert(msg);
+
+       // check request type, that should be always DLOG_REQ_HANDLE_LOGUTIL
+       // as dispatched by service_writer_handle_req_ctrl handler
+       // don't assert for compatibility with service_writer_handle_req_pipe
+       // and possible mistakes in the future that would be hard to track
+       if (msg->request != DLOG_REQ_HANDLE_LOGUTIL)
+               return -EINVAL;
+
        if (msg->length <= sizeof(struct dlog_control_msg) ||
            msg->length >= LOG_MAX_SIZE - 1)
                return -EINVAL;
@@ -1259,6 +1298,7 @@ static int service_writer_handle_req_util(struct logger* server, struct writer*
        if (r < 0)
                return r;
 
+       assert(server);
        if (!server->buffers[rd->buf_id]) {
                reader_free(rd);
                return -EINVAL;
@@ -1268,6 +1308,7 @@ static int service_writer_handle_req_util(struct logger* server, struct writer*
        if (r < 0)
                return r;
 
+       assert(wr);
        if (wr->readed > msg->length) {
                wr->readed -= msg->length;
                memcpy(wr->buffer, wr->buffer + msg->length, wr->readed);
@@ -1295,6 +1336,15 @@ static void apply_reader_clear(void *ptr, void *user_data)
  */
 static int service_writer_handle_req_clear(struct logger* server, struct writer* wr, struct dlog_control_msg* msg)
 {
+       assert(msg);
+
+       // check request type, that should be always DLOG_REQ_CLEAR
+       // as dispatched by service_writer_handle_req_ctrl handler
+       // don't assert for compatibility with service_writer_handle_req_pipe
+       // and possible mistakes in the future that would be hard to track
+       if (msg->request != DLOG_REQ_CLEAR)
+               return -EINVAL;
+
        if (msg->length != (sizeof(struct dlog_control_msg)))
                return -EINVAL;
 
@@ -1304,6 +1354,7 @@ static int service_writer_handle_req_clear(struct logger* server, struct writer*
        wr->buf_ptr->head = wr->buf_ptr->tail = 0;
        wr->buf_ptr->lines = 0;
 
+       assert(server);
        list_foreach(server->buffers[wr->buf_ptr->id]->readers, NULL, apply_reader_clear);
 
        if (wr->readed > msg->length) {
@@ -1316,6 +1367,30 @@ static int service_writer_handle_req_clear(struct logger* server, struct writer*
 }
 
 /**
+ * @brief Service control request
+ * @details Handle a clear-buffer or util request in respect to msg request type
+ * @param[in] server The logger server
+ * @param[in] wr The writer who sent the request
+ * @param[in] msg The message containing the request
+ * @return 0 on success, else -errno
+ */
+static int service_writer_handle_req_ctrl(struct logger *server, struct writer *wr, struct dlog_control_msg *msg)
+{
+       assert(msg);
+
+       switch (msg->request) {
+       case DLOG_REQ_CLEAR:
+               return service_writer_handle_req_clear(server, wr, msg);
+               break;
+       case DLOG_REQ_HANDLE_LOGUTIL:
+               return service_writer_handle_req_util(server, wr, msg);
+               break;
+       default:
+               return -EINVAL;
+       }
+}
+
+/**
  * @brief Service a pipe acquisition request
  * @details Handle a pipe request
  * @param[in] server The logger server
@@ -1326,6 +1401,14 @@ static int service_writer_handle_req_clear(struct logger* server, struct writer*
 static int service_writer_handle_req_pipe(struct logger* server, struct writer* wr, struct dlog_control_msg* msg)
 {
        int r;
+
+       assert(msg);
+
+       // check request type given by user
+       // don't assert that as the message is not parsed before
+       if (msg->request != DLOG_REQ_PIPE)
+               return -EINVAL;
+
        if (msg->length != sizeof(struct dlog_control_msg))
                return -EINVAL;
 
@@ -1333,11 +1416,13 @@ static int service_writer_handle_req_pipe(struct logger* server, struct writer*
        if (pipe2(pipe_fd, O_CLOEXEC | O_NONBLOCK) < 0)
                return -errno;
 
+       assert(wr);
        wr->event.data.ptr = wr;
        wr->event.events = EPOLLIN;
        fd_clear_flags(pipe_fd[1], O_NONBLOCK);
        fcntl(pipe_fd[1], F_SETPIPE_SZ, PIPE_REQUESTED_SIZE);
 
+       assert(server);
        r = add_fd_loop(server->epollfd, pipe_fd[0], &wr->event);
        if (r < 0)
                goto err_close;
@@ -1372,8 +1457,11 @@ err_close:
 static int service_writer_socket(struct logger* server, struct writer* wr, struct epoll_event* event)
 {
        int r = 0;
+
+       assert(wr);
        struct dlog_control_msg* const msg = (struct dlog_control_msg*) wr->buffer;
 
+       assert(event);
        if (event->events & EPOLLIN)
                r = read(wr->working_fd, wr->buffer + wr->readed, LOG_MAX_SIZE - wr->readed);
 
@@ -1390,27 +1478,8 @@ static int service_writer_socket(struct logger* server, struct writer* wr, struc
                if (wr->readed < msg->length)
                        continue;
 
-               if (wr->rights) {
-                       switch (msg->request) {
-                       case DLOG_REQ_CLEAR:
-                               r = service_writer_handle_req_clear(server, wr, msg);
-                               break;
-                       case DLOG_REQ_HANDLE_LOGUTIL:
-                               r = service_writer_handle_req_util(server, wr, msg);
-                               break;
-                       default:
-                               return -EINVAL;
-                       }
-               } else {
-                       switch (msg->request) {
-                       case DLOG_REQ_PIPE:
-                               r = service_writer_handle_req_pipe(server, wr, msg);
-                               break;
-                       default:
-                               return -EINVAL;
-                       }
-               }
-
+               assert(wr->service_socket);
+               r = wr->service_socket(server, wr, msg);
                if (r <= 0)
                        return r;
 
@@ -1570,7 +1639,7 @@ static void service_all_readers(struct logger* server, int force_push)
  * @param[in] epoll_fd Epoll FD to manage the socket
  * @return 0 on success, else -errno
  */
-int socket_initialize(struct sock_data * sock, struct log_buffer * buffer, int type, struct socket_config_data * data, int epoll_fd)
+int socket_initialize(struct sock_data *sock, struct log_buffer *buffer, service_socket_t service_socket, struct socket_config_data *data, int epoll_fd)
 {
        if ((sock->fd = listen_fd_create(data->path, data->permissions)) < 0)
                return sock->fd;
@@ -1579,7 +1648,8 @@ int socket_initialize(struct sock_data * sock, struct log_buffer * buffer, int t
        if (r < 0)
                return r;
 
-       sock->_entity.type = type;
+       sock->_entity.dispatch_event = dispatch_event_sock;
+       sock->service_socket = service_socket;
        sock->event.data.ptr = sock;
        sock->buf_ptr = buffer;
        sock->event.events = EPOLLIN;
@@ -1597,6 +1667,9 @@ int socket_initialize(struct sock_data * sock, struct log_buffer * buffer, int t
  */
 static void logger_add_writer(struct logger* l, struct writer* wr)
 {
+       assert(l);
+       assert(wr);
+
        list_add(&l->writers, wr);
        add_fd_loop(l->epollfd, wr->working_fd, &wr->event);
 }
@@ -1668,43 +1741,47 @@ static void logger_free(struct logger* l)
 }
 
 /**
- * @brief Dispatch event
+ * @brief Dispatch writer event
  * @details Receives and handles an event
  * @param[in] server The logger server
- * @param[in] entity The entity who received the event
  * @param[in] event The received event
  */
-static void dispatch_event(struct logger* server, struct fd_entity* entity, struct epoll_event* event)
-{
-       switch (entity->type) {
-       case ENTITY_WRITER: {
-               struct writer *writer = (struct writer*)entity;
-               assert(writer->service_writer);
-               int r = writer->service_writer(server, writer, event);
-               if (r) {
-                       list_remove(&server->writers, writer);
-                       writer_free(writer);
-               }
-               break;
-       }
-       case ENTITY_SOCK_CONTROL:
-       case ENTITY_SOCK_WRITE: {
-               struct sock_data const * const sock = (struct sock_data const * const) event->data.ptr;
-               int sock_pipe = accept4(sock->fd, NULL, NULL, SOCK_NONBLOCK);
-               if (sock_pipe >= 0) {
-                       struct writer *writer;
-                       if (writer_create(&writer, sock_pipe, entity->type == ENTITY_SOCK_CONTROL,
-                                                         sock->buf_ptr, service_writer_socket) == 0)
-                               logger_add_writer(server, writer);
-                       else
-                               close(sock_pipe);
-               }
-               break;
-       }
+static void dispatch_event_writer(struct logger *server, struct epoll_event *event)
+{
+       assert(server);
+       assert(event);
+       struct writer *writer = (struct writer*)event->data.ptr;
+
+       assert(writer->service_writer);
+       int r = writer->service_writer(server, writer, event);
+       if (r) {
+               list_remove(&server->writers, writer);
+               writer_free(writer);
        }
 }
 
 /**
+ * @brief Dispatch socket event
+ * @details Receives and handles an event
+ * @param[in] server The logger server
+ * @param[in] event The received event
+ */
+static void dispatch_event_sock(struct logger *server, struct epoll_event *event)
+{
+       assert(event);
+       struct sock_data const * const sock = (struct sock_data const * const) event->data.ptr;
+       int sock_pipe = accept4(sock->fd, NULL, NULL, SOCK_NONBLOCK);
+       if (sock_pipe < 0)
+               return;
+
+       struct writer *writer;
+       if (writer_create(&writer, sock_pipe, sock->buf_ptr, service_writer_socket, sock->service_socket) == 0)
+               logger_add_writer(server, writer);
+       else
+               close(sock_pipe);
+}
+
+/**
  * @brief Handle interrupting/terminating signals
  * @details Clears global flag to stop main loop
  * @param[in] signo signal number
@@ -1748,7 +1825,8 @@ static int do_logger(struct logger* server)
 
                for (i = 0; i < nfds; i++) {
                        struct fd_entity* entity = (struct fd_entity*) events[i].data.ptr;
-                       dispatch_event(server, entity, &events[i]);
+                       assert(entity->dispatch_event);
+                       entity->dispatch_event(server, &events[i]);
                }
                service_all_readers(server, nfds == 0);
        }