Move all pipe backend filtering server-side

Previously, we used to carry around two filter settings objects in
dlogutil: a filter list, which contained a simple text list of all
filterspecs, and a filter object, which contained info about all other
filters set. The logger backend merged those two together and used
them to filter everything locally, while the pipe backend sent the
filter list to the server and filtered locally on filter object. This
patch does away with the filter list, so that everything is in the
filter object, and enables client to send the filter object. In
particular, this means that the server is able to filter by TID and PID
now.

Change-Id: Ic962da04a5657527f2f80fe20e35caa75d7e2782

diff --git a/include/fd_info.h b/include/fd_info.h
index c8bb599..89e5e20 100644 (file)
--- a/include/fd_info.h
+++ b/include/fd_info.h
@@ -54,13 +54,16 @@ struct fd_ops {
 
        /// Transition into a state that allows reading and printing out logs
        /// Return values > 0 mean everything went fine but we don't want to print
-       int (*prepare_print)(struct fd_info *fdi, int dump, list_head filter_list, log_filter *filter_object);
+       int (*prepare_print)(struct fd_info *fdi, int dump, log_filter *filter_object);
 
        /// Clear the buffer
        int (*clear)(struct fd_info *fdi);
 
        /// Get the size of the buffer
        int (*getsize)(struct fd_info *fdi, uint32_t *val);
+
+       // Whether to filter logs on the client
+       bool should_filter;
 };
 
 struct fd_info *fdi_create(struct fd_ops *ops, const char *name);

diff --git a/include/logprint.h b/include/logprint.h
index fd98343..5ccd526 100644 (file)
--- a/include/logprint.h
+++ b/include/logprint.h
@@ -24,6 +24,7 @@
 #include <pthread.h>
 
 #include <logcommon.h>
+#include <ptrs_list.h>
 #include <queued_entry.h>
 
 #ifdef __cplusplus
@@ -121,6 +122,33 @@ int log_add_filter_pid(log_filter *p_filter, pid_t pid);
 
 int log_add_filter_tid(log_filter *p_filter, pthread_t tid);
 
+void log_filter_clear(log_filter *p_filter);
+
+struct tag_and_prio {
+       char *tag;
+       size_t tagLength;
+       log_priority pri;
+       bool exactPri;
+       bool prefix;
+};
+
+typedef struct FilterInfo_t {
+       enum {
+               FILTER_TAG_AND_PRIO,
+               FILTER_PID,
+               FILTER_TID,
+       } type;
+       union {
+               struct tag_and_prio tnp;
+               pid_t pid;
+               pid_t tid;
+       };
+} FilterInfo;
+
+list_head log_filter_get_list(log_filter *p_filter);
+
+log_priority log_filter_get_global_priority(log_filter *p_filter, bool *is_exact);
+
 /**
  * returns true if this log line should be printed based on its entry
  * data (priority, tag, pid and tid), and false if it should not

diff --git a/include/logretrieve.h b/include/logretrieve.h
index 144d4c4..084cef7 100644 (file)
--- a/include/logretrieve.h
+++ b/include/logretrieve.h
@@ -4,7 +4,6 @@
 #include <logconfig.h>
 
 struct additional_options {
-       list_head filter_list;
        log_filter *filter_object;
        size_t logs_dump;
        size_t logs_size;

diff --git a/src/logger/logger.c b/src/logger/logger.c
index 19d2f2f..16d5e03 100644 (file)
--- a/src/logger/logger.c
+++ b/src/logger/logger.c
@@ -19,6 +19,7 @@
 #endif
 
 #include "logger_internal.h"
+#include <getopt.h>
 
 /**
  * @addtogroup DLOG_IMPLEMENTATION
@@ -1049,7 +1050,13 @@ static int parse_command_line(const char *cmdl, struct writer *wr, struct logger
        reader->log_storage_reader = NULL;
        clock_gettime(CLOCK_MONOTONIC, &reader->last_read_time);
 
-       while ((option = getopt(argc, argv, "cdt:gsf:r:n:v:b:u:")) != -1) {
+       static const struct option long_options[] = {
+               {"tid", required_argument, NULL, 0},
+               {"pid", required_argument, NULL, 1},
+               {0}
+       };
+
+       while ((option = getopt_long(argc, argv, "cdt:gsf:r:n:v:b:u:", long_options, NULL)) != -1) {
                switch (option) {
                        break;
                case 'd':
@@ -1096,6 +1103,18 @@ static int parse_command_line(const char *cmdl, struct writer *wr, struct logger
                        silence = 1;
                        log_add_filter_string(reader->filter, "*:s");
                        break;
+               case 0: {
+                       pid_t tid;
+                       if (sscanf(optarg, "%d", &tid) != 1 || log_add_filter_tid(reader->filter, tid))
+                               goto cleanup;
+                       break;
+               }
+               case 1: {
+                       pid_t pid;
+                       if (sscanf(optarg, "%d", &pid) != 1 || log_add_filter_pid(reader->filter, pid))
+                               goto cleanup;
+                       break;
+               }
                case '?': // invalid option or missing argument
                        retval = -EINVAL;
                        goto cleanup;

diff --git a/src/logutil/logutil.c b/src/logutil/logutil.c
index f4c52b4..e7df690 100644 (file)
--- a/src/logutil/logutil.c
+++ b/src/logutil/logutil.c
@@ -61,6 +61,8 @@ int parse_options(int argc, char **argv, struct log_file *l_file, int *enabled_b
        assert(action);
        assert(opt);
 
+       bool change_default = true;
+
        while (1) {
                static const struct option long_options[] = {
                        {"tid"    , required_argument, NULL,   0},
@@ -150,7 +152,7 @@ int parse_options(int argc, char **argv, struct log_file *l_file, int *enabled_b
                        break;
                }
                case 's':
-                       list_add(&opt->filter_list, "*:s");
+                       change_default = false;
                        break;
                case 'r':
                        if (sscanf(optarg, "%zu", &l_file->rotate_size_kbytes) != 1)
@@ -174,8 +176,13 @@ int parse_options(int argc, char **argv, struct log_file *l_file, int *enabled_b
                }
        }
 
-       while (optind < argc)
-               list_add(&opt->filter_list, argv[optind++]);
+       while (optind < argc) {
+               log_add_filter_string(opt->filter_object, argv[optind++]);
+               change_default = false;
+       }
+
+       if (change_default)
+               log_add_filter_string(opt->filter_object, "*:D");
 
        return 0;
 

diff --git a/src/shared/fd_info.c b/src/shared/fd_info.c
index 5ab9848..5911fa8 100644 (file)
--- a/src/shared/fd_info.c
+++ b/src/shared/fd_info.c
@@ -85,7 +85,7 @@ int fdi_push_log(struct fd_info *fdi, struct sort_vector *logs, write_callback c
        if (!temp)
                return -ENOMEM;
 
-       if (!log_should_print_line(filter, temp)) {
+       if (fdi->ops->should_filter && !log_should_print_line(filter, temp)) {
                free(temp);
                return 0;
        }

diff --git a/src/shared/fdi_logger.c b/src/shared/fdi_logger.c
index 7c26bc4..9a1322a 100644 (file)
--- a/src/shared/fdi_logger.c
+++ b/src/shared/fdi_logger.c
@@ -159,16 +159,13 @@ static void logger_destroy(struct fd_info *fdi)
        fdi->priv_data = NULL;
 }
 
-static int logger_prepare_print(struct fd_info *fdi, int dump, list_head filter_list, log_filter *filter_object)
+static int logger_prepare_print(struct fd_info *fdi, int dump, log_filter *filter_object)
 {
        assert(fdi);
        struct logger_priv_data *const lpd = (struct logger_priv_data *)fdi->priv_data;
        assert(lpd);
        assert(filter_object);
 
-       for (list_head iter = filter_list; iter; list_next(&iter))
-               log_add_filter_string(filter_object, (const char *)list_at(iter));
-
        if (dump == DUMP_CONTINUOUS) {
                lpd->log_len = UNLIMITED_LOG_LEN;
                return 0;
@@ -267,4 +264,5 @@ struct fd_ops ops_logger = {
        .prepare_print = logger_prepare_print,
        .clear = logger_clear,
        .getsize = logger_getsize,
+       .should_filter = true,
 };

diff --git a/src/shared/fdi_pipe.c b/src/shared/fdi_pipe.c
index 24d278a..bee3d0d 100644 (file)
--- a/src/shared/fdi_pipe.c
+++ b/src/shared/fdi_pipe.c
@@ -77,7 +77,7 @@ static int pipe_clear(struct fd_info *fdi)
  * @param[in] sock_fd Socket file descriptor
  * @return 0 on success, -errno on failure
  */
-static int send_logger_request(list_head filters, int dump, int sock_fd)
+static int send_logger_request(log_filter *filters, int dump, int sock_fd)
 {
        assert(filters);
        assert(sock_fd >= 0);
@@ -91,18 +91,53 @@ static int send_logger_request(list_head filters, int dump, int sock_fd)
                len += sizeof dump_str - 1;
        }
 
-       for (list_head iter = filters; iter; list_next(&iter)) {
-               const char *filter = (const char *)list_at(iter);
-               int arglen = strnlen(filter, MAX_LOGGER_REQUEST_LEN);
-               int needed = arglen + 1 /* space */;
+       char single_option[MAX_LOGGER_REQUEST_LEN];
+       int needed = 0;
+
+       for (list_head iter = log_filter_get_list(filters); iter; list_next(&iter)) {
+               const FilterInfo *const filter = list_at(iter);
+
+               switch (filter->type) {
+                       case FILTER_TAG_AND_PRIO:
+                               needed = snprintf(single_option, sizeof single_option, " %s%s:%s%c",
+                                       filter->tnp.tag,
+                                       filter->tnp.prefix ? "*" : "",
+                                       filter->tnp.exactPri ? "=" : "",
+                                       filter_pri_to_char(filter->tnp.pri));
+                               break;
+                       case FILTER_TID:
+                               needed = snprintf(single_option, sizeof single_option, " --tid %d", filter->tid);
+                               break;
+                       case FILTER_PID:
+                               needed = snprintf(single_option, sizeof single_option, " --pid %d", filter->pid);
+                               break;
+               }
+
+               if (needed == -1)
+                       return -errno;
+
                if (len + needed > sizeof(request_string))
                        return -E2BIG;
 
-               strncat(request_string, " ", 1);
-               strncat(request_string, filter, arglen + 1);
+               strncat(request_string, single_option, needed);
                len += needed;
        }
 
+       bool global_exact;
+       log_priority global_prio = log_filter_get_global_priority(filters, &global_exact);
+       needed = snprintf(single_option, sizeof single_option, " *:%s%c",
+               global_exact ? "=" : "",
+               filter_pri_to_char(global_prio));
+
+       if (needed == -1)
+               return -errno;
+
+       if (len + needed > sizeof(request_string))
+               return -E2BIG;
+
+       strncat(request_string, single_option, needed);
+       len += needed;
+
        return send_dlog_request(sock_fd, DLOG_REQ_HANDLE_LOGUTIL, request_string, len);
 }
 
@@ -157,18 +192,15 @@ static void pipe_destroy(struct fd_info *fdi)
        fdi->priv_data = NULL;
 }
 
-static int pipe_prepare_print(struct fd_info *fdi, int dump, list_head filter_list, log_filter *filter_object)
+static int pipe_prepare_print(struct fd_info *fdi, int dump, log_filter *filter_object)
 {
-       assert(filter_list);
        assert(filter_object);
        assert(fdi);
        struct pipe_priv_data *ppd = (struct pipe_priv_data *)fdi->priv_data;
        assert(ppd);
        assert(ppd->sock_fd >= 0);
 
-       log_add_filter_string(filter_object, "*:V"); // lets everything through (actual filtering done by the daemon)
-
-       int r = send_logger_request(filter_list, dump, ppd->sock_fd);
+       int r = send_logger_request(filter_object, dump, ppd->sock_fd);
        if (r < 0)
                return r;
 
@@ -252,4 +284,5 @@ struct fd_ops ops_pipe = {
        .prepare_print = pipe_prepare_print,
        .clear = pipe_clear,
        .getsize = pipe_getsize,
+       .should_filter = false,
 };

diff --git a/src/shared/logprint.c b/src/shared/logprint.c
index c3afa2f..4d53445 100644 (file)
--- a/src/shared/logprint.c
+++ b/src/shared/logprint.c
@@ -23,7 +23,6 @@
 #include <ctype.h>
 
 #include <logprint.h>
-#include <ptrs_list.h>
 
 /**
  * @addtogroup SHARED_FUNCTIONS
@@ -33,27 +32,6 @@
 #define FILTERINFO_PID_NONE -1
 #define FILTERINFO_TID_NONE -1
 
-struct tag_and_prio {
-       char *tag;
-       int tagLength;
-       log_priority pri;
-       bool exactPri;
-       bool prefix;
-};
-
-typedef struct FilterInfo_t {
-       enum {
-               FILTER_TAG_AND_PRIO,
-               FILTER_PID,
-               FILTER_TID,
-       } type;
-       union {
-               struct tag_and_prio tnp;
-               pid_t pid;
-               pid_t tid;
-       };
-} FilterInfo;
-
 struct log_filter_t {
        list_head filters;
        log_priority global_pri;
@@ -338,12 +316,37 @@ log_filter *log_filter_from_filter(const log_filter *p_filter)
 void log_filter_free(log_filter *p_filter)
 {
        if (p_filter) {
-               list_clear_custom(&p_filter->filters, NULL, filterinfo_free_cb);
+               log_filter_clear(p_filter);
 
                free(p_filter);
        }
 }
 
+void log_filter_clear(log_filter *p_filter)
+{
+       assert(p_filter);
+
+       list_clear_custom(&p_filter->filters, NULL, filterinfo_free_cb);
+       p_filter->global_pri = DLOG_SILENT;
+       p_filter->exact_global_pri = false;
+}
+
+list_head log_filter_get_list(log_filter *p_filter)
+{
+       assert(p_filter);
+
+       return p_filter->filters;
+}
+
+log_priority log_filter_get_global_priority(log_filter *p_filter, bool *is_exact)
+{
+       assert(is_exact);
+       assert(p_filter);
+
+       *is_exact = p_filter->exact_global_pri;
+       return p_filter->global_pri;
+}
+
 /**
  * @brief Print format from string
  * @details Converts a string to print format

diff --git a/src/shared/logretrieve.c b/src/shared/logretrieve.c
index 260697e..f0d2da8 100644 (file)
--- a/src/shared/logretrieve.c
+++ b/src/shared/logretrieve.c
@@ -81,7 +81,6 @@ failure:
 
 int additional_options_init(struct additional_options *opt)
 {
-       opt->filter_list = NULL;
        opt->filter_object = log_filter_new();
        opt->logs_dump = DUMP_CONTINUOUS;
        opt->logs_size = DEFAULT_SORT_BUFFER_SIZE;
@@ -90,7 +89,6 @@ int additional_options_init(struct additional_options *opt)
 
 void additional_options_cleanup(struct additional_options *opt)
 {
-       list_clear(&opt->filter_list);
        log_filter_free(opt->filter_object);
 }
 
@@ -154,9 +152,6 @@ int do_print(struct fd_info **data_fds, int fd_count, write_callback callback, v
        assert(data_fds);
        assert(userdata);
 
-       if (opt->filter_list == NULL)
-               list_add(&opt->filter_list, "*:D");
-
        int epoll_cnt = 0;
        __attribute__((cleanup(close_fd))) int epollfd = epoll_create1(0);
        if (epollfd < 0)
@@ -173,7 +168,7 @@ int do_print(struct fd_info **data_fds, int fd_count, write_callback callback, v
                return -ENOMEM;
 
        for (int nfds = 0; nfds < fd_count; ++nfds) {
-               int r = data_fds[nfds]->ops->prepare_print(data_fds[nfds], logs.dump, opt->filter_list, opt->filter_object);
+               int r = data_fds[nfds]->ops->prepare_print(data_fds[nfds], logs.dump, opt->filter_object);
                if (r < 0)
                        return r;
                if (r > 0) {

diff --git a/src/tests/fd_info.c b/src/tests/fd_info.c
index 46f2774..9c8e4e8 100644 (file)
--- a/src/tests/fd_info.c
+++ b/src/tests/fd_info.c
@@ -91,6 +91,7 @@ int main()
        struct fd_ops fops = {
                .destroy = test_destroy,
                .extract_entry = test_extract,
+               .should_filter = true,
        };
 
        malloc_ret = NULL;

diff --git a/src/tests/fdi_logger.c b/src/tests/fdi_logger.c
index 56cc3b8..a2aad59 100644 (file)
--- a/src/tests/fdi_logger.c
+++ b/src/tests/fdi_logger.c
@@ -179,24 +179,15 @@ int main()
        assert(0 == ops_logger.getsize(&fdi, &sz));
        assert(2019 == sz);
 
-       list_head filters = NULL;
        expected_ioctl = -1;
-       assert(!ops_logger.prepare_print(&fdi, DUMP_CONTINUOUS, filters, (log_filter *) 0xCA5CADE));
+       assert(!ops_logger.prepare_print(&fdi, DUMP_CONTINUOUS, (log_filter *) 0xCA5CADE));
 
        expected_ioctl = LOGGER_GET_LOG_LEN;
        ioctl_ret = -67;
-       assert(-67 == ops_logger.prepare_print(&fdi, 123, filters, (log_filter *) 0xCA5CADE));
+       assert(-67 == ops_logger.prepare_print(&fdi, 123, (log_filter *) 0xCA5CADE));
        expected_ioctl = LOGGER_GET_LOG_LEN;
        ioctl_ret = 0;
-       assert(1 == ops_logger.prepare_print(&fdi, 123, filters, (log_filter *) 0xCA5CADE));
-
-       ioctl_ret = 128;
-       filters_added = 0;
-       for (size_t i = 0; i < NELEMS(filter_strs); ++i)
-               list_add(&filters, filter_strs[i]);
-
-       assert(!ops_logger.prepare_print(&fdi, 123, filters, (log_filter *) 0xCA5CADE));
-       assert(filters_added == NELEMS(filter_strs));
+       assert(1 == ops_logger.prepare_print(&fdi, 123, (log_filter *) 0xCA5CADE));
 
        fail_read = true;
        assert(-334 == ops_logger.read(&fdi));

diff --git a/src/tests/fdi_pipe.c b/src/tests/fdi_pipe.c
index d70d3a6..3abd958 100644 (file)
--- a/src/tests/fdi_pipe.c
+++ b/src/tests/fdi_pipe.c
@@ -134,38 +134,44 @@ int main()
 
        log_filter *filter = log_filter_new();
        assert(filter);
-       list_head filters = NULL;
-       list_add(&filters, (void *)"filter0");
+       log_add_filter_string(filter, "filter0");
 
        correct_sockfd = 2;
        correct_request = DLOG_REQ_HANDLE_LOGUTIL;
-       correct_send_data = "dlogutil filter0";
+       correct_send_data = "dlogutil filter0:V *:S";
        correct_send_datalen = strlen(correct_send_data) + 1;
        send_return = -1;
-       assert(ops_pipe.prepare_print(&info, false, filters, filter) == -1);
+       assert(ops_pipe.prepare_print(&info, false, filter) == -1);
 
-       correct_send_data = "dlogutil -d filter0";
+       correct_send_data = "dlogutil -d filter0:V *:S";
        correct_send_datalen = strlen(correct_send_data) + 1;
-       assert(ops_pipe.prepare_print(&info, true, filters, filter) == -1);
+       assert(ops_pipe.prepare_print(&info, true, filter) == -1);
 
-       list_add(&filters, (void *)"filter1");
-       correct_send_data = "dlogutil filter1 filter0";
+       log_add_filter_string(filter, "filter1");
+       correct_send_data = "dlogutil filter1:V filter0:V *:S";
        correct_send_datalen = strlen(correct_send_data) + 1;
-       assert(ops_pipe.prepare_print(&info, false, filters, filter) == -1);
+       assert(ops_pipe.prepare_print(&info, false, filter) == -1);
+
+       log_add_filter_tid(filter, 123);
+       log_add_filter_pid(filter, 456);
+       correct_send_data = "dlogutil --pid 456 --tid 123 filter1:V filter0:V *:S";
+       correct_send_datalen = strlen(correct_send_data) + 1;
+       assert(ops_pipe.prepare_print(&info, false, filter) == -1);
 
        send_return = 0;
        recv_pipe_fail = true;
-       assert(ops_pipe.prepare_print(&info, false, filters, filter) == -EIO);
+       assert(ops_pipe.prepare_print(&info, false, filter) == -EIO);
 
        recv_pipe_fail = false;
-       assert(ops_pipe.prepare_print(&info, false, filters, filter) == 0);
+       correct_send_data = "dlogutil --pid 456 --tid 123 filter1:V filter0:V *:S";
+       correct_send_datalen = strlen(correct_send_data) + 1;
+       assert(ops_pipe.prepare_print(&info, false, filter) == 0);
        assert(info.fd == 3);
 
-       list_add(&filters, (void *)"This is a long and complicated filter. In fact, it's way too long to be accepted by ops_pipe.prepare_print, which should return -E2BIG.");
-       assert(ops_pipe.prepare_print(&info, false, filters, filter) == -E2BIG);
+       log_add_filter_string(filter, "This is a long and complicated filter. In fact, it's way too long to be accepted by ops_pipe.prepare_print, which should return -E2BIG.");
+       assert(ops_pipe.prepare_print(&info, false, filter) == -E2BIG);
 
        log_filter_free(filter);
-       list_clear(&filters);
 
        correct_sockfd = 3;