First round of fixes based on cppcheck

https://github.com/fish-shell/fish-shell/issues/575

10 files changed, 49 insertions, 64 deletions

diff --git a/builtin.cpp b/builtin.cpp
index 007adc12..619fc20a 100644
--- a/builtin.cpp
+++ b/builtin.cpp
@@ -1171,7 +1171,7 @@ static void functions_def(const wcstring &name, wcstring &out)
 
 
     wcstring_list_t named = function_get_named_arguments(name);
-    if (named.size() > 0)
+    if (! named.empty())
     {
         append_format(out, L" --argument");
         for (size_t i=0; i < named.size(); i++)
@@ -3531,7 +3531,7 @@ static int builtin_end(parser_t &parser, wchar_t **argv)
                     const wcstring val = for_vars.back();
                     for_vars.pop_back();
                     const wcstring &for_variable = fb->variable;
-                    env_set(for_variable.c_str(), val.c_str(),  ENV_LOCAL);
+                    env_set(for_variable, val.c_str(),  ENV_LOCAL);
                     parser.current_block->loop_status = LOOP_NORMAL;
                     parser.current_block->skip = 0;
 
diff --git a/builtin_complete.cpp b/builtin_complete.cpp
index ecfcceaf..b4ac2a38 100644
--- a/builtin_complete.cpp
+++ b/builtin_complete.cpp
@@ -98,7 +98,7 @@ static void  builtin_complete_add2(const wchar_t *cmd,
                      flags);
     }
 
-    if (old_opt.size() == 0 && gnu_opt.size() == 0 && wcslen(short_opt) == 0)
+    if (old_opt.empty() && gnu_opt.empty() && wcslen(short_opt) == 0)
     {
         complete_add(cmd,
                      cmd_type,
@@ -204,7 +204,7 @@ static void  builtin_complete_remove2(const wchar_t *cmd,
     {
         for (; *s; s++)
         {
-            if (old_opt.size() == 0 && gnu_opt.size() == 0)
+            if (old_opt.empty() && gnu_opt.empty())
             {
                 complete_remove(cmd,
                                 cmd_type,
diff --git a/builtin_set.cpp b/builtin_set.cpp
index a094dd1c..dca0dbc5 100644
--- a/builtin_set.cpp
+++ b/builtin_set.cpp
@@ -125,7 +125,7 @@ static int my_env_set(const wchar_t *key, const wcstring_list_t &val, int scope)
     }
 
     wcstring sb;
-    if (val.size())
+    if (! val.empty())
     {
         for (i=0; i< val.size() ; i++)
         {
@@ -313,7 +313,7 @@ static void erase_values(wcstring_list_t &list, const std::vector<long> &indexes
 
     // Now walk the set backwards, so we encounter larger indexes first, and remove elements at the given (1-based) indexes.
     std::set<long>::const_reverse_iterator iter;
-    for (iter = indexes_set.rbegin(); iter != indexes_set.rend(); iter++)
+    for (iter = indexes_set.rbegin(); iter != indexes_set.rend(); ++iter)
     {
         long val = *iter;
         if (val > 0 && (size_t)val <= list.size())
diff --git a/common.cpp b/common.cpp
index afc7430c..91646b8d 100644
--- a/common.cpp
+++ b/common.cpp
@@ -112,11 +112,10 @@ void show_stackframe()
         return;
 
     void *trace[32];
-    char **messages = (char **)NULL;
     int i, trace_size = 0;
 
     trace_size = backtrace(trace, 32);
-    messages = backtrace_symbols(trace, trace_size);
+    char **messages = backtrace_symbols(trace, trace_size);
 
     if (messages)
     {
diff --git a/complete.cpp b/complete.cpp
index 669fd574..60abfc67 100644
--- a/complete.cpp
+++ b/complete.cpp
@@ -860,7 +860,7 @@ int complete_is_valid_option(const wcstring &str,
                     if (errors)
                     {
                         const wcstring str = opt.substr(j, 1);
-                        errors->push_back(format_error(_(L"Unknown option: "), str.c_str()));
+                        errors->push_back(format_error(_(L"Unknown option: "), str));
                     }
 
                     opt_found = 0;
@@ -1375,7 +1375,7 @@ bool completer_t::complete_param(const wcstring &scmd_orig, const wcstring &spop
     /* Now release the lock and test each option that we captured above.
        We have to do this outside the lock because callouts (like the condition) may add or remove completions.
        See https://github.com/ridiculousfish/fishfish/issues/2 */
-    for (std::vector<local_options_t>::const_iterator iter = all_options.begin(); iter != all_options.end(); iter++)
+    for (std::vector<local_options_t>::const_iterator iter = all_options.begin(); iter != all_options.end(); ++iter)
     {
         const option_list_t &options = iter->options;
         use_common=1;
diff --git a/env.cpp b/env.cpp
index 43113990..6f4ab8e9 100644
--- a/env.cpp
+++ b/env.cpp
@@ -1467,7 +1467,7 @@ static void update_export_array_if_necessary(bool recalc)
         for (i=0; i<uni.size(); i++)
         {
             const wcstring &key = uni.at(i);
-            const wchar_t *val = env_universal_get(key.c_str());
+            const wchar_t *val = env_universal_get(key);
 
             if (wcscmp(val, ENV_NULL))
             {
diff --git a/env_universal.cpp b/env_universal.cpp
index 28180b6b..bd3350b8 100644
--- a/env_universal.cpp
+++ b/env_universal.cpp
@@ -46,7 +46,7 @@
 #define RECONNECT_COUNT 32
 
 
-connection_t env_universal_server;
+connection_t env_universal_server(-1);
 
 /**
    Set to true after initialization has been performed
@@ -280,8 +280,6 @@ void env_universal_init(wchar_t * p,
     start_fishd=sf;
     external_callback = cb;
 
-    connection_init(&env_universal_server, -1);
-
     env_universal_server.fd = get_socket();
     env_universal_common_init(&callback);
     env_universal_read_all();
@@ -372,7 +370,7 @@ void env_universal_barrier()
     */
     msg= create_message(BARRIER, 0, 0);
     msg->count=1;
-    env_universal_server.unsent->push(msg);
+    env_universal_server.unsent.push(msg);
 
     /*
       Wait until barrier request has been sent
@@ -383,7 +381,7 @@ void env_universal_barrier()
         try_send_all(&env_universal_server);
         check_connection();
 
-        if (env_universal_server.unsent->empty())
+        if (env_universal_server.unsent.empty())
             break;
 
         if (env_universal_server.fd == -1)
@@ -445,7 +443,7 @@ void env_universal_set(const wcstring &name, const wcstring &value, bool exportv
         }
 
         msg->count=1;
-        env_universal_server.unsent->push(msg);
+        env_universal_server.unsent.push(msg);
         env_universal_barrier();
     }
 }
@@ -473,7 +471,7 @@ int env_universal_remove(const wchar_t *name)
     {
         msg= create_message(ERASE, name, 0);
         msg->count=1;
-        env_universal_server.unsent->push(msg);
+        env_universal_server.unsent.push(msg);
         env_universal_barrier();
     }
 
diff --git a/env_universal_common.cpp b/env_universal_common.cpp
index 52df1ae8..1aa169bf 100644
--- a/env_universal_common.cpp
+++ b/env_universal_common.cpp
@@ -595,21 +595,16 @@ static void parse_message(wchar_t *msg,
         tmp = wcschr(name, L':');
         if (tmp)
         {
-            wchar_t *key;
             wchar_t *val;
-
-            key = (wchar_t *)malloc(sizeof(wchar_t)*(tmp-name+1));
-            memcpy(key, name, sizeof(wchar_t)*(tmp-name));
-            key[tmp-name]=0;
-
+            const wcstring key(name, tmp - name);
+            
             val = tmp+1;
             val = unescape(val, 0);
 
-            if (key && val)
-                env_universal_common_set(key, val, exportv);
+            if (val != NULL)
+                env_universal_common_set(key.c_str(), val, exportv);
 
             free(val);
-            free(key);
         }
         else
         {
@@ -646,7 +641,7 @@ static void parse_message(wchar_t *msg,
     {
         message_t *msg = create_message(BARRIER_REPLY, 0, 0);
         msg->count = 1;
-        src->unsent->push(msg);
+        src->unsent.push(msg);
         try_send_all(src);
     }
     else if (match(msg, BARRIER_REPLY_STR))
@@ -716,12 +711,12 @@ void try_send_all(connection_t *c)
     /*  debug( 3,
            L"Send all updates to connection on fd %d",
            c->fd );*/
-    while (!c->unsent->empty())
+    while (!c->unsent.empty())
     {
-        switch (try_send(c->unsent->front(), c->fd))
+        switch (try_send(c->unsent.front(), c->fd))
         {
             case 1:
-                c->unsent->pop();
+                c->unsent.pop();
                 break;
 
             case 0:
@@ -930,25 +925,23 @@ void enqueue_all(connection_t *c)
 
         message_t *msg = create_message(entry.exportv ? SET_EXPORT : SET, key.c_str(), entry.val.c_str());
         msg->count=1;
-        c->unsent->push(msg);
+        c->unsent.push(msg);
     }
 
     try_send_all(c);
 }
 
-
-void connection_init(connection_t *c, int fd)
+connection_t::connection_t(const int input_fd) :
+    fd(input_fd),
+    killme(false),
+    buffer_consumed(0),
+    buffer_used(0),
+    next(NULL)
 {
-    memset(c, 0, sizeof(connection_t));
-    c->fd = fd;
-    c->unsent = new std::queue<message_t *>;
-    c->buffer_consumed = c->buffer_used = 0;
 }
 
 void connection_destroy(connection_t *c)
 {
-    if (c->unsent) delete c->unsent;
-
     /*
       A connection need not always be open - we only try to close it
       if it is open.
diff --git a/env_universal_common.h b/env_universal_common.h
index 21fcd8eb..e5f44f76 100644
--- a/env_universal_common.h
+++ b/env_universal_common.h
@@ -76,20 +76,22 @@ typedef std::queue<message_t *> message_queue_t;
 /**
    This struct represents a connection between a universal variable server/client
 */
-typedef struct connection
+struct connection_t
 {
     /**
        The file descriptor this socket lives on
     */
     int fd;
+    
     /**
-       Queue of onsent messages
+       Queue of unsent messages
     */
-    message_queue_t *unsent;
+    std::queue<message_t *> unsent;
+
     /**
        Set to one when this connection should be killed
     */
-    int killme;
+    bool killme;
     /**
        The input string. Input from the socket goes here. When a
        newline is encountered, the buffer is parsed and cleared.
@@ -111,13 +113,14 @@ typedef struct connection
     */
     size_t buffer_used;
 
-
     /**
        Link to the next connection
     */
-    struct connection *next;
-}
-connection_t;
+    struct connection_t *next;
+    
+    /* Constructor */
+    connection_t(int input_fd);
+};
 
 /**
    Read all available messages on this connection
@@ -194,12 +197,6 @@ bool env_universal_common_get_export(const wcstring &name);
 void enqueue_all(connection_t *c);
 
 /**
-   Fill in the specified connection_t struct. Use the specified file
-   descriptor for communication.
-*/
-void connection_init(connection_t *c, int fd);
-
-/**
    Close and destroy the specified connection struct. This frees
    allstructures allocated by the connection, such as ques of unsent
    messages.
diff --git a/fishd.cpp b/fishd.cpp
index d1364a89..8023b9e8 100644
--- a/fishd.cpp
+++ b/fishd.cpp
@@ -638,7 +638,7 @@ static void broadcast(fish_message_type_t type, const wchar_t *key, const wchar_
     for (c = conn; c; c=c->next)
     {
         msg->count++;
-        c->unsent->push(msg);
+        c->unsent.push(msg);
     }
 
     for (c = conn; c; c=c->next)
@@ -787,8 +787,7 @@ static bool load_or_save_variables_at_path(bool save, const std::string &path)
     {
         /* Success */
         result = true;
-        connection_t c = {};
-        connection_init(&c, fd);
+        connection_t c(fd);
 
         if (save)
         {
@@ -962,7 +961,7 @@ int main(int argc, char ** argv)
             FD_SET(c->fd, &read_fd);
             max_fd = maxi(max_fd, c->fd+1);
 
-            if (! c->unsent->empty())
+            if (! c->unsent.empty())
             {
                 FD_SET(c->fd, &write_fd);
             }
@@ -1009,8 +1008,7 @@ int main(int argc, char ** argv)
                 }
                 else
                 {
-                    connection_t *newc = (connection_t *)malloc(sizeof(connection_t));
-                    connection_init(newc, child_socket);
+                    connection_t *newc = new connection_t(child_socket);
                     newc->next = conn;
                     send(newc->fd, GREETING, strlen(GREETING), MSG_DONTWAIT);
                     enqueue_all(newc);
@@ -1055,10 +1053,10 @@ int main(int argc, char ** argv)
             {
                 debug(4, L"Close connection %d", c->fd);
 
-                while (! c->unsent->empty())
+                while (! c->unsent.empty())
                 {
-                    message_t *msg = c->unsent->front();
-                    c->unsent->pop();
+                    message_t *msg = c->unsent.front();
+                    c->unsent.pop();
                     msg->count--;
                     if (!msg->count)
                         free(msg);
@@ -1074,7 +1072,7 @@ int main(int argc, char ** argv)
                     conn=c->next;
                 }
 
-                free(c);
+                delete c;
 
                 c=(prev?prev->next:conn);