From 4416753df0116452891f6c389712b60b4ea219b6 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 16 Feb 2013 02:38:13 -0800 Subject: [PATCH] More cleanup based on static analysis https://github.com/fish-shell/fish-shell/issues/575 --- env_universal_common.cpp | 14 +++---- exec.cpp | 2 +- fishd.cpp | 82 +++++++++++++++++----------------------- parse_util.cpp | 2 +- reader.cpp | 5 +-- screen.cpp | 2 +- wildcard.cpp | 4 +- 7 files changed, 47 insertions(+), 64 deletions(-) diff --git a/env_universal_common.cpp b/env_universal_common.cpp index 6887f8cf9..dbda23775 100644 --- a/env_universal_common.cpp +++ b/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; - msg->count = 0; - - char *key=0; + char *key = NULL; // 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; } diff --git a/exec.cpp b/exec.cpp index f05bef7a9..fbbc88254 100644 --- a/exec.cpp +++ b/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 &tmp_io = *iter; if (tmp_io->io_mode == IO_FILE && strcmp(static_cast(tmp_io.get())->filename_cstr, "/dev/null") != 0) diff --git a/fishd.cpp b/fishd.cpp index ada93f01c..4ae08c5c4 100644 --- a/fishd.cpp +++ b/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; - pw = getpwuid(getuid()); - uname = strdup(pw->pw_name); + const struct passwd *pw = getpwuid(getuid()); + uname = pw->pw_name; } - name = (char *)malloc(strlen(dir)+ strlen(uname)+ strlen(SOCK_FILENAME) + 2); - if (name == NULL) - { - wperror(L"get_socket_filename"); - exit(EXIT_FAILURE); - } - strcpy(name, dir); - strcat(name, "/"); - strcat(name, SOCK_FILENAME); - strcat(name, uname); + std::string name; + name.reserve(strlen(dir)+ strlen(uname)+ strlen(SOCK_FILENAME) + 1); + name.append(dir); + name.push_back('/'); + name.append(SOCK_FILENAME); + name.append(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); - char *lockfile = (char *)malloc(len + strlen(LOCKPOSTFIX) + 1); - - if (lockfile == NULL) + bool success = false; + std::string lockfile; + lockfile.reserve(sock_name.size() + strlen(LOCKPOSTFIX)); + lockfile = sock_name; + lockfile.append(LOCKPOSTFIX); + if (acquire_lock_file(lockfile, LOCKTIMEOUT, 1)) { - wperror(L"acquire_socket_lock"); - exit(EXIT_FAILURE); + out_lockfile_name->swap(lockfile); + success = true; } - strcpy(lockfile, sock_name); - strcpy(lockfile + len, LOCKPOSTFIX); - if (!acquire_lock_file(lockfile, LOCKTIMEOUT, 1)) - { - free(lockfile); - lockfile = NULL; - } - return lockfile; + return success; } /** 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); - if (lockfile == NULL) + std::string lockfile; + 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); - debug(4, L"Released lockfile: %s", lockfile); + (void)unlink(lockfile.c_str()); + 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; diff --git a/parse_util.cpp b/parse_util.cpp index 11abd912d..a7614f0c2 100644 --- a/parse_util.cpp +++ b/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; diff --git a/reader.cpp b/reader.cpp index f8fe26192..a2b7cfd82 100644 --- a/reader.cpp +++ b/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.size() > 0) + if (! lst.empty()) { writestr(L"\x1b]0;"); - for (i=0; i &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++) { diff --git a/wildcard.cpp b/wildcard.cpp index 8f48848f3..f8e1bed00 100644 --- a/wildcard.cpp +++ b/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 &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)) {