From e48cb7c89ff3e54de70130a3de2136a9902a023d Mon Sep 17 00:00:00 2001 From: Sergei Trofimovich Date: Fri, 30 Jan 2009 09:31:28 +0200 Subject: [PATCH] mhl: added mhl_strmove() function (memmove semantics) valgrind detected an error in completion path: ==2962== Source and destination overlap in strcpy(0x459F068, 0x459F06A) ==2962== at 0x4026056: strcpy (mc_replace_strmem.c:268) ==2962== by 0x808F70B: canonicalize_pathname (string3.h:106) ==2962== by 0x805ECBA: filename_completion_function (complete.c:125) ==2962== by 0x805FB35: command_completion_function (complete.c:448) ==2962== by 0x805EA34: completion_matches (complete.c:552) ==2962== by 0x8060454: complete (complete.c:735) ==2962== by 0x809AAC4: handle_char (widget.c:1545) ==2962== by 0x807867E: midnight_callback (dialog.h:201) ==2962== by 0x8061B27: dlg_process_event (dialog.c:664) ==2962== by 0x8061ECE: run_dlg (dialog.c:786) ==2962== by 0x807996C: main (main.c:1674) Snippet of man strcpy: DESCRIPTION The strcpy() function copies the string pointed to by src, including the terminating null byte ('\0'), to the buffer pointed to by dest. ___The strings may not overlap___, and the destination string dest must be large enough to receive the copy. We used strcpy to move data chunk in memory: "./foo" -> "foo", etc. This patch introduces mhl_strmove and fixed canonicalize_pathname. Signed-off-by: Sergei Trofimovich --- mhl/string.h | 13 +++++++++++++ src/utilunix.c | 12 +++++++----- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/mhl/string.h b/mhl/string.h index e3c7d2f26..a6e0eee37 100644 --- a/mhl/string.h +++ b/mhl/string.h @@ -3,6 +3,7 @@ #include #include +#include #include #define mhl_str_dup(str) ((str ? strdup(str) : strdup(""))) @@ -123,4 +124,16 @@ static inline char* mhl_str_reverse(char* ptr) return ptr; } +/* + * strcpy is unsafe on overlapping memory areas, so define memmove-alike + * string function. Has sense only when dest <= src. + */ +static inline char * mhl_strmove(char * dest, const char * src) +{ + size_t n = strlen (src) + 1; /* + '\0' */ + + assert (dest<=src); + + return memmove(dest, src, n); +} #endif /* __MHL_STRING_H */ diff --git a/src/utilunix.c b/src/utilunix.c index 7565eb1ab..1b72b2e40 100644 --- a/src/utilunix.c +++ b/src/utilunix.c @@ -41,6 +41,8 @@ #endif #include +#include + #include "global.h" #include "execute.h" #include "wtools.h" /* message() */ @@ -426,7 +428,7 @@ canonicalize_pathname (char *path) if (p[0] == PATH_SEP && p[1] == PATH_SEP) { s = p + 1; while (*(++s) == PATH_SEP); - strcpy (p + 1, s); + mhl_strmove (p + 1, s); } p++; } @@ -435,7 +437,7 @@ canonicalize_pathname (char *path) p = lpath; while (*p) { if (p[0] == PATH_SEP && p[1] == '.' && p[2] == PATH_SEP) - strcpy (p, p + 2); + mhl_strmove (p, p + 2); else p++; } @@ -451,7 +453,7 @@ canonicalize_pathname (char *path) lpath[1] = 0; return; } else { - strcpy (lpath, lpath + 2); + mhl_strmove (lpath, lpath + 2); } } @@ -497,10 +499,10 @@ canonicalize_pathname (char *path) if (p[3] != 0) { if (s == lpath && *s == PATH_SEP) { /* "/../foo" -> "/foo" */ - strcpy (s + 1, p + 4); + mhl_strmove (s + 1, p + 4); } else { /* "token/../foo" -> "foo" */ - strcpy (s, p + 4); + mhl_strmove (s, p + 4); } p = (s > lpath) ? s - 1 : s; continue;