More cleanup based on static analysis

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

env_universal_common.cpp

@@ -487,7 +487,7 @@ void read_message(connection_t *src)
             {
                 src->killme = 1;
                 debug(3, L"Fd %d has reached eof, set killme flag", src->fd);
-                if (src->input.size() > 0)
+                if (! src->input.empty())
                 {
                     char c = 0;
                     src->input.push_back(c);
@@ -799,10 +799,7 @@ message_t *create_message(fish_message_type_t type,
                           const wchar_t *key_in,
                           const wchar_t *val_in)
 {
-    message_t *msg = new message_t;
+    char *key = NULL;
-    msg->count = 0;
-
-    char *key=0;
 
     //  debug( 4, L"Crete message of type %d", type );
 
@@ -811,7 +808,7 @@ message_t *create_message(fish_message_type_t type,
         if (wcsvarname(key_in))
         {
             debug(0, L"Illegal variable name: '%ls'", key_in);
-            return 0;
+            return NULL;
         }
 
         key = wcs2utf(key_in);
@@ -820,10 +817,12 @@ message_t *create_message(fish_message_type_t type,
             debug(0,
                   L"Could not convert %ls to narrow character string",
                   key_in);
-            return 0;
+            return NULL;
         }
     }
 
+    message_t *msg = new message_t;
+    msg->count = 0;
 
     switch (type)
     {
@@ -839,7 +838,6 @@ message_t *create_message(fish_message_type_t type,
             char *val = wcs2utf(esc.c_str());
             set_body(msg, (type==SET?SET_MBS:SET_EXPORT_MBS), " ", key, ":", val, "\n", NULL);
             free(val);
-
             break;
         }
 

exec.cpp

@@ -1191,7 +1191,7 @@ void exec(parser_t &parser, job_t *j)
                     skip_fork = true;
                 }
 
-                for (io_chain_t::iterator iter = j->io.begin(); iter != j->io.end(); iter++)
+                for (io_chain_t::iterator iter = j->io.begin(); iter != j->io.end(); ++iter)
                 {
                     const shared_ptr<io_data_t> &tmp_io = *iter;
                     if (tmp_io->io_mode == IO_FILE && strcmp(static_cast<const io_file_t *>(tmp_io.get())->filename_cstr, "/dev/null") != 0)

fishd.cpp

@@ -157,9 +157,8 @@ static int quit=0;
 /**
    Constructs the fish socket filename
 */
-static char *get_socket_filename()
+static std::string get_socket_filename(void)
 {
-    char *name;
     const char *dir = getenv("FISHD_SOCKET_DIR");
     char *uname = getenv("USER");
 
@@ -170,25 +169,20 @@ static char *get_socket_filename()
 
     if (uname == NULL)
     {
-        struct passwd *pw;
+        const struct passwd *pw = getpwuid(getuid());
-        pw = getpwuid(getuid());
+        uname = pw->pw_name;
-        uname = strdup(pw->pw_name);
     }
 
-    name = (char *)malloc(strlen(dir)+ strlen(uname)+ strlen(SOCK_FILENAME) + 2);
+    std::string name;
-    if (name == NULL)
+    name.reserve(strlen(dir)+ strlen(uname)+ strlen(SOCK_FILENAME) + 1);
-    {
+    name.append(dir);
-        wperror(L"get_socket_filename");
+    name.push_back('/');
-        exit(EXIT_FAILURE);
+    name.append(SOCK_FILENAME);
-    }
+    name.append(uname);
-    strcpy(name, dir);
-    strcat(name, "/");
-    strcat(name, SOCK_FILENAME);
-    strcat(name, uname);
 
-    if (strlen(name) >= UNIX_PATH_MAX)
+    if (name.size() >= UNIX_PATH_MAX)
     {
-        debug(1, L"Filename too long: '%s'", name);
+        debug(1, L"Filename too long: '%s'", name.c_str());
         exit(EXIT_FAILURE);
     }
     return name;
@@ -256,7 +250,7 @@ static void sprint_rand_digits(char *str, int maxlen)
  fallback.
  The memory returned should be freed using free().
  */
-static std::string gen_unique_nfs_filename(const char *filename)
+static std::string gen_unique_nfs_filename(const std::string &filename)
 {
     char hostname[HOST_NAME_MAX + 1];
     char pid_str[256];
@@ -403,7 +397,7 @@ static std::string get_machine_identifier(void)
  A unique temporary file named by appending characters to the lockfile name
  is used; any pre-existing file of the same name is subject to deletion.
  */
-static int acquire_lock_file(const char *lockfile, const int timeout, int force)
+static int acquire_lock_file(const std::string &lockfile_str, const int timeout, int force)
 {
     int fd, timed_out = 0;
     int ret = 0; /* early exit returns failure */
@@ -411,6 +405,7 @@ static int acquire_lock_file(const char *lockfile, const int timeout, int force)
     struct timeval start, end;
     double elapsed;
     struct stat statbuf;
+    const char * const lockfile = lockfile_str.c_str();
 
     /*
        (Re)create a unique file and check that it has one only link.
@@ -514,52 +509,47 @@ done:
    The returned string must be free()d after unlink()ing the file to release
    the lock
 */
-static char *acquire_socket_lock(const char *sock_name)
+static bool acquire_socket_lock(const std::string &sock_name, std::string *out_lockfile_name)
 {
-    size_t len = strlen(sock_name);
+    bool success = false;
-    char *lockfile = (char *)malloc(len + strlen(LOCKPOSTFIX) + 1);
+    std::string lockfile;
-
+    lockfile.reserve(sock_name.size() + strlen(LOCKPOSTFIX));
-    if (lockfile == NULL)
+    lockfile = sock_name;
+    lockfile.append(LOCKPOSTFIX);
+    if (acquire_lock_file(lockfile, LOCKTIMEOUT, 1))
     {
-        wperror(L"acquire_socket_lock");
+        out_lockfile_name->swap(lockfile);
-        exit(EXIT_FAILURE);
+        success = true;
     }
-    strcpy(lockfile, sock_name);
+    return success;
-    strcpy(lockfile + len, LOCKPOSTFIX);
-    if (!acquire_lock_file(lockfile, LOCKTIMEOUT, 1))
-    {
-        free(lockfile);
-        lockfile = NULL;
-    }
-    return lockfile;
 }
 
 /**
    Connects to the fish socket and starts listening for connections
 */
-static int get_socket()
+static int get_socket(void)
 {
     int s, len, doexit = 0;
     int exitcode = EXIT_FAILURE;
     struct sockaddr_un local;
-    char *sock_name = get_socket_filename();
+    const std::string sock_name = get_socket_filename();
 
     /*
        Start critical section protected by lock
     */
-    char *lockfile = acquire_socket_lock(sock_name);
+    std::string lockfile;
-    if (lockfile == NULL)
+    if (! acquire_socket_lock(sock_name, &lockfile))
     {
         debug(0, L"Unable to obtain lock on socket, exiting");
         exit(EXIT_FAILURE);
     }
-    debug(4, L"Acquired lockfile: %s", lockfile);
+    debug(4, L"Acquired lockfile: %s", lockfile.c_str());
 
     local.sun_family = AF_UNIX;
-    strcpy(local.sun_path, sock_name);
+    strcpy(local.sun_path, sock_name.c_str());
     len = sizeof(local);
 
-    debug(1, L"Connect to socket at %s", sock_name);
+    debug(1, L"Connect to socket at %s", sock_name.c_str());
 
     if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) == -1)
     {
@@ -601,19 +591,15 @@ static int get_socket()
     }
 
 unlock:
-    (void)unlink(lockfile);
+    (void)unlink(lockfile.c_str());
-    debug(4, L"Released lockfile: %s", lockfile);
+    debug(4, L"Released lockfile: %s", lockfile.c_str());
     /*
        End critical section protected by lock
     */
 
-    free(lockfile);
-
-    free(sock_name);
-
     if (doexit)
     {
-        exit(exitcode);
+        exit_without_destructors(exitcode);
     }
 
     return s;

parse_util.cpp

@@ -564,7 +564,7 @@ void parse_util_set_argv(const wchar_t * const *argv, const wcstring_list_t &nam
         env_set(L"argv", 0, ENV_LOCAL);
     }
 
-    if (named_arguments.size())
+    if (! named_arguments.empty())
     {
         const wchar_t * const *arg;
         size_t i;

reader.cpp

@@ -735,11 +735,10 @@ void reader_write_title()
     proc_push_interactive(0);
     if (exec_subshell(title, lst, false /* do not apply exit status */) != -1)
     {
-        size_t i;
+        if (! lst.empty())
-        if (lst.size() > 0)
         {
             writestr(L"\x1b]0;");
-            for (i=0; i<lst.size(); i++)
+            for (size_t i=0; i<lst.size(); i++)
             {
                 writestr(lst.at(i).c_str());
             }

screen.cpp

@@ -1049,7 +1049,7 @@ struct screen_layout_t
 /* Given a vector whose indexes are offsets and whose values are the widths of the string if truncated at that offset, return the offset that fits in the given width. Returns width_by_offset.size() - 1 if they all fit. The first value in width_by_offset is assumed to be 0. */
 static size_t truncation_offset_for_width(const std::vector<size_t> &width_by_offset, size_t max_width)
 {
-    assert(width_by_offset.size() > 0 && width_by_offset.at(0) == 0);
+    assert(! width_by_offset.empty() && width_by_offset.at(0) == 0);
     size_t i;
     for (i=1; i < width_by_offset.size(); i++)
     {

wildcard.cpp

@@ -338,7 +338,7 @@ static wcstring complete_get_desc_suffix_internal(const wcstring &suff)
 
     if (exec_subshell(cmd, lst, false /* do not apply exit status */) != -1)
     {
-        if (lst.size()>0)
+        if (! lst.empty())
         {
             const wcstring & ln = lst.at(0);
             if (ln.size() > 0 && ln != L"unknown")
@@ -620,7 +620,7 @@ static void wildcard_completion_allocate(std::vector<completion_t> &list,
     bool wants_desc = !(expand_flags & EXPAND_NO_DESCRIPTIONS);
     wcstring desc;
     if (wants_desc)
-        desc = file_get_desc(fullname.c_str(), lstat_res, lbuf, stat_res, buf, stat_errno);
+        desc = file_get_desc(fullname, lstat_res, lbuf, stat_res, buf, stat_errno);
 
     if (sz >= 0 && S_ISDIR(buf.st_mode))
     {