lib: fix static analysis issues, use regfree()

* Fix potential NULL dereference
* Fix use of uninitialized value
* Fix leaking memory by not freeing regex_t
* Fix extra \n when using empty regex filter
* Clean up still-reachable hook memory
* Handle nonexistent pager

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>

diff --git a/lib/command.c b/lib/command.c
index 3bd578cf0139437abf32c846236369852ad1c5c0..edf7348ba75e29dff33dca4aa7074aebb46507ae 100644 (file)
--- a/lib/command.c
+++ b/lib/command.c
@@ -1198,16 +1198,22 @@ static int handle_pipe_action(struct vty *vty, const char *cmd_in,
        if (strmatch(token, "include")) {
                /* the remaining text should be a regexp */
                char *regexp = working;
+
+               if (!regexp) {
+                       vty_out(vty, "%% Need a regexp to filter with\n");
+                       goto fail;
+               }
+
                bool succ = vty_set_include(vty, regexp);
 
                if (!succ) {
-                       vty_out(vty, "%% Bad regexp '%s'", regexp);
+                       vty_out(vty, "%% Bad regexp '%s'\n", regexp);
                        goto fail;
                }
                *cmd_out = XSTRDUP(MTYPE_TMP, cmd_in);
                *(strstr(*cmd_out, "|")) = '\0';
        } else {
-               vty_out(vty, "%% Unknown action '%s'", token);
+               vty_out(vty, "%% Unknown action '%s'\n", token);
                goto fail;
        }
 
@@ -2892,6 +2898,9 @@ void cmd_terminate()
 {
        struct cmd_node *cmd_node;
 
+       hook_unregister(cmd_execute, handle_pipe_action);
+       hook_unregister(cmd_execute_done, handle_pipe_action_done);
+
        if (cmdvec) {
                for (unsigned int i = 0; i < vector_active(cmdvec); i++)
                        if ((cmd_node = vector_slot(cmdvec, i)) != NULL) {

diff --git a/lib/frrstr.c b/lib/frrstr.c
index 03368b3665317ee61986bd32b5570c69131c19e6..d003590ba6d2b0937bf000bf53da618059d43226 100644 (file)
--- a/lib/frrstr.c
+++ b/lib/frrstr.c
@@ -58,6 +58,9 @@ vector frrstr_split_vec(const char *string, const char *delimiter)
        char **result;
        int argc;
 
+       if (!string)
+               return NULL;
+
        frrstr_split(string, delimiter, &result, &argc);
 
        vector v = array_to_vector((void **)result, argc);
@@ -89,7 +92,7 @@ char *frrstr_join(const char **parts, int argc, const char *join)
 
                memcpy(p, parts[i], arglen);
                p += arglen;
-               if (i + 1 != argc) {
+               if (i + 1 != argc && join) {
                        memcpy(p, join, joinlen);
                        p += joinlen;
                }

diff --git a/lib/vty.c b/lib/vty.c
index 0bdcbf22843b373b0f3dce5575a04f0e242ae208..803b1c408d5ce03181692ba363d418148dfcfa0c 100644 (file)
--- a/lib/vty.c
+++ b/lib/vty.c
@@ -119,8 +119,8 @@ bool vty_set_include(struct vty *vty, const char *regexp)
        bool ret = true;
        char errbuf[256];
 
-       if (!regexp) {
-               memset(&vty->include, 0x00, sizeof(vty->include));
+       if (!regexp && vty->filter) {
+               regfree(&vty->include);
                vty->filter = false;
                return true;
        }
@@ -188,8 +188,26 @@ int vty_out(struct vty *vty, const char *format, ...)
                vector lines = frrstr_split_vec(buf, "\n");
 
                frrstr_filter_vec(lines, &vty->include);
-               if (buf[strlen(buf) - 1] == '\n' && vector_active(lines) > 0)
+
+               /*
+                * Consider the string "foo\n". If the regex is an empty string
+                * and the line ended with a newline, then the vector will look
+                * like:
+                *
+                * [0]: 'foo'
+                * [1]: ''
+                *
+                * If the regex isn't empty, the vector will look like:
+                *
+                * [0]: 'foo'
+                *
+                * In this case we'd like to preserve the newline, so we add
+                * the empty string [1] as in the first example.
+                */
+               if (buf[strlen(buf) - 1] == '\n' && vector_active(lines) > 0
+                   && strlen(vector_slot(lines, vector_active(lines) - 1)))
                        vector_set(lines, XSTRDUP(MTYPE_TMP, ""));
+
                filtered = frrstr_join_vec(lines, "\n");
                frrstr_strvec_free(lines);
        } else {

diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c
index cd47ef1c4538fa6ce64091792450d8cfc52d55d8..d28c879d5760a14491c07345a6cab015ca69e948 100644 (file)
--- a/vtysh/vtysh.c
+++ b/vtysh/vtysh.c
@@ -70,6 +70,9 @@ static FILE *vty_open_pager(struct vty *vty)
        if (vty->is_paged)
                return vty->of;
 
+       if (!vtysh_pager_name)
+               return NULL;
+
        vty->of_saved = vty->of;
        vty->of = popen(vtysh_pager_name, "w");
        if (vty->of == NULL) {