From db1ec847f989a71b2cb1e0c39194e277613931fd Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Wed, 1 Jun 2016 22:03:27 -0700 Subject: fix lint error in wgettext() Cppcheck was complaining about the `return val.c_str()` at the end of the `wgettext()` function. That would normally a bug since the lifetime of `val` ends when the function returns. In this particular case that's not true because the string is interned in a cache. Nonetheless, rather than suppress the lint warning I decided to modify the API to be more idiomatic. In the process of fixing the aforementioned lint warning I fixed several other lint errors in that module. This required making our copy of `wgetopt()` compatible with the rest of the fish code. Specifically, by removing its local definitions of the "_" macro so it uses the same macro used everywhere else in the fish code. The sooner we kill the use of wide chars the better. --- src/common.h | 4 ++-- src/complete.cpp | 11 +++++++---- src/wgetopt.cpp | 22 +++------------------- src/wutil.cpp | 35 ++++++++++++++++++----------------- src/wutil.h | 2 +- 5 files changed, 31 insertions(+), 43 deletions(-) (limited to 'src') diff --git a/src/common.h b/src/common.h index e59dfc0e..07342436 100644 --- a/src/common.h +++ b/src/common.h @@ -226,8 +226,8 @@ void write_ignore(int fd, const void *buff, size_t count); return retval; \ } -/// Shorthand for wgettext call. -#define _(wstr) wgettext(wstr) +/// Shorthand for wgettext call in situations where a C-style string is needed (e.g., fwprintf()). +#define _(wstr) wgettext(wstr).c_str() /// Noop, used to tell xgettext that a string should be translated, even though it is not directly /// sent to wgettext. diff --git a/src/complete.cpp b/src/complete.cpp index 6062a055..f686ab31 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -56,7 +56,9 @@ /// since it can occur, and should not be translated. (Gettext returns the version information as /// the response). #ifdef USE_GETTEXT -static const wchar_t *C_(const wcstring &s) { return s.empty() ? L"" : wgettext(s.c_str()); } +static const wchar_t *C_(const wcstring &s) { + return s.empty() ? L"" : wgettext(s.c_str()).c_str(); +} #else static const wcstring &C_(const wcstring &s) { return s; } #endif @@ -193,12 +195,13 @@ completion_t::~completion_t() {} // Clear the COMPLETE_AUTO_SPACE flag, and set COMPLETE_NO_SPACE appropriately depending on the // suffix of the string. static complete_flags_t resolve_auto_space(const wcstring &comp, complete_flags_t flags) { + complete_flags_t new_flags = flags; if (flags & COMPLETE_AUTO_SPACE) { - flags = flags & ~COMPLETE_AUTO_SPACE; + new_flags &= ~COMPLETE_AUTO_SPACE; size_t len = comp.size(); - if (len > 0 && (wcschr(L"/=@:", comp.at(len - 1)) != 0)) flags |= COMPLETE_NO_SPACE; + if (len > 0 && (wcschr(L"/=@:", comp.at(len - 1)) != 0)) new_flags |= COMPLETE_NO_SPACE; } - return flags; + return new_flags; } // completion_t functions. Note that the constructor resolves flags! diff --git a/src/wgetopt.cpp b/src/wgetopt.cpp index f7ed5427..f31b623b 100644 --- a/src/wgetopt.cpp +++ b/src/wgetopt.cpp @@ -36,7 +36,7 @@ // You should have received a copy of the GNU Library General Public License along with the GNU C // Library; see the file COPYING.LIB. If not, write to the Free Software Foundation, Inc., 675 Mass // Ave, Cambridge, MA 02139, USA. -#include "config.h" +#include "config.h" // IWYU pragma: keep #include #include @@ -62,21 +62,6 @@ #include "wgetopt.h" #include "wutil.h" // IWYU pragma: keep -// Use translation functions if available. -#ifdef _ -#undef _ -#endif - -#ifdef HAVE_TRANSLATE_H -#ifdef USE_GETTEXT -#define _(string) wgettext(string) -#else -#define _(string) (string) -#endif -#else -#define _(wstr) wstr -#endif - #ifdef __GNU_LIBRARY__ // We want to avoid inclusion of string.h with non-GNU libraries because there are many ways it can // cause trouble. On some systems, it contains special magic macros that don't work in GCC. @@ -306,9 +291,8 @@ int wgetopter_t::_wgetopt_internal(int argc, wchar_t **argv, const wchar_t *opts int indfound = 0; // set to zero by Anton int option_index; - for (nameend = nextchar; *nameend && *nameend != '='; nameend++) { - // Do nothing. - } + for (nameend = nextchar; *nameend && *nameend != '='; nameend++) + ; //!OCLINT(empty body) // Test all long options for either exact match or abbreviated matches. for (p = longopts, option_index = 0; p->name; p++, option_index++) diff --git a/src/wutil.cpp b/src/wutil.cpp index 02e975b2..5e1ecea8 100644 --- a/src/wutil.cpp +++ b/src/wutil.cpp @@ -166,7 +166,6 @@ FILE *wfopen(const wcstring &path, const char *mode) { default: { errno = EINVAL; return NULL; - break; } } // Skip binary. @@ -196,18 +195,22 @@ bool set_cloexec(int fd) { static int wopen_internal(const wcstring &pathname, int flags, mode_t mode, bool cloexec) { ASSERT_IS_NOT_FORKED_CHILD(); cstring tmp = wcs2string(pathname); -// Prefer to use O_CLOEXEC. It has to both be defined and nonzero. + int fd; + #ifdef O_CLOEXEC - if (cloexec && (O_CLOEXEC != 0)) { - flags |= O_CLOEXEC; - cloexec = false; + // Prefer to use O_CLOEXEC. It has to both be defined and nonzero. + if (cloexec) { + fd = open(tmp.c_str(), flags | O_CLOEXEC, mode); + } else { + fd = open(tmp.c_str(), flags, mode); } -#endif - int fd = ::open(tmp.c_str(), flags, mode); - if (cloexec && fd >= 0 && !set_cloexec(fd)) { +#else + fd = open(tmp.c_str(), flags, mode); + if (fd >= 0 && !set_cloexec(fd)) { close(fd); fd = -1; } +#endif return fd; } @@ -251,7 +254,8 @@ void wperror(const wchar_t *s) { int make_fd_nonblocking(int fd) { int flags = fcntl(fd, F_GETFL, 0); int err = 0; - if (!(flags & O_NONBLOCK)) { + bool nonblocking = flags & O_NONBLOCK; + if (!nonblocking) { err = fcntl(fd, F_SETFL, flags | O_NONBLOCK); } return err == -1 ? errno : 0; @@ -260,7 +264,8 @@ int make_fd_nonblocking(int fd) { int make_fd_blocking(int fd) { int flags = fcntl(fd, F_GETFL, 0); int err = 0; - if (flags & O_NONBLOCK) { + bool nonblocking = flags & O_NONBLOCK; + if (nonblocking) { err = fcntl(fd, F_SETFL, flags & ~O_NONBLOCK); } return err == -1 ? errno : 0; @@ -406,17 +411,13 @@ static void wgettext_init_if_necessary() { pthread_once(&once, wgettext_really_init); } -const wchar_t *wgettext(const wchar_t *in) { - if (!in) return in; - +const wcstring &wgettext(const wchar_t *in) { // Preserve errno across this since this is often used in printing error messages. int err = errno; + wcstring key = in; wgettext_init_if_necessary(); - - wcstring key = in; scoped_lock lock(wgettext_lock); //!OCLINT(has side effects) - wcstring &val = wgettext_map[key]; if (val.empty()) { cstring mbs_in = wcs2string(key); @@ -427,7 +428,7 @@ const wchar_t *wgettext(const wchar_t *in) { // The returned string is stored in the map. // TODO: If we want to shrink the map, this would be a problem. - return val.c_str(); + return val; } int wmkdir(const wcstring &name, int mode) { diff --git a/src/wutil.h b/src/wutil.h index 9fc61096..b5c77870 100644 --- a/src/wutil.h +++ b/src/wutil.h @@ -84,7 +84,7 @@ std::wstring wbasename(const std::wstring &path); /// gettext function, wgettext takes care of setting the correct domain, etc. using the textdomain /// and bindtextdomain functions. This should probably be moved out of wgettext, so that wgettext /// will be nothing more than a wrapper around gettext, like all other functions in this file. -const wchar_t *wgettext(const wchar_t *in); +const wcstring &wgettext(const wchar_t *in); /// Wide character version of mkdir. int wmkdir(const wcstring &dir, int mode); -- cgit v1.2.3