diff options
author | Stephen Gallagher <sgallagh@redhat.com> | 2012-03-07 09:29:46 -0500 |
---|---|---|
committer | Stephen Gallagher <sgallagh@redhat.com> | 2012-03-08 08:10:56 -0500 |
commit | 1f9961c46d1d507046e57bb08e0d2e25d77f2d82 (patch) | |
tree | 6ce0e6cae33c3b9b5a92567d47d1d22e0767a83f | |
parent | 030d99cef35962dc78111407218d66215296de4b (diff) | |
download | ding-libs-1f9961c46d1d507046e57bb08e0d2e25d77f2d82.tar.gz ding-libs-1f9961c46d1d507046e57bb08e0d2e25d77f2d82.tar.xz ding-libs-1f9961c46d1d507046e57bb08e0d2e25d77f2d82.zip |
path_utils: path_concat should return empty string on ENOBUFS
Also clean up the code a bit and add more comments
-rw-r--r-- | path_utils/path_utils.c | 48 | ||||
-rw-r--r-- | path_utils/path_utils.h | 3 | ||||
-rw-r--r-- | path_utils/path_utils_ut.c | 19 |
3 files changed, 55 insertions, 15 deletions
diff --git a/path_utils/path_utils.c b/path_utils/path_utils.c index 25ca426..0a758c5 100644 --- a/path_utils/path_utils.c +++ b/path_utils/path_utils.c @@ -192,6 +192,7 @@ bool is_absolute_path(const char *path) int path_concat(char *path, size_t path_size, const char *head, const char *tail) { + int ret; const char *p, *src; char *dst, *dst_end; @@ -201,13 +202,27 @@ int path_concat(char *path, size_t path_size, const char *head, const char *tail dst_end = path + path_size - 1; /* -1 allows for NULL terminator */ if (head && *head) { - for (p = head; *p; p++); /* walk to end of head */ - for (p--; p > head && *p == '/'; p--); /* skip any trailing slashes in head */ - if ((p - head) > path_size-1) return ENOBUFS; - for (src = head; src <= p && dst < dst_end;) *dst++ = *src++; /* copy head */ + /* walk to end of head */ + for (p = head; *p; p++); + + /* skip any trailing slashes in head */ + for (p--; p > head && *p == '/'; p--); + + /* If the length of head exceeds the buffer size, fail */ + if ((p - head) > path_size-1) { + ret = ENOBUFS; + goto fail; + } + + /* Copy head into path */ + for (src = head; src <= p && dst < dst_end;) { + *dst++ = *src++; + } } if (tail && *tail) { - for (p = tail; *p && *p == '/'; p++); /* skip any leading slashes in tail */ + /* skip any leading slashes in tail */ + for (p = tail; *p && *p == '/'; p++); + if (dst > path) /* insert single slash between head & tail * Making sure not to add an extra if the @@ -218,15 +233,28 @@ int path_concat(char *path, size_t path_size, const char *head, const char *tail if (dst < dst_end && *(dst-1) != '/') { *dst++ = '/'; } - for (src = p; *src && dst < dst_end;) *dst++ = *src++; /* copy tail */ - if (*src) return ENOBUFS; /* failed to copy everything */ + + /* Copy the tail into the path */ + for (src = p; *src && dst < dst_end;) { + *dst++ = *src++; + } + + /* If we got past dst_end and there is more data + * in the src buffer, we should return ENOBUFS + */ + if (*src) { + ret = ENOBUFS; /* failed to copy everything */ + goto fail; + } } *dst = 0; - if (dst > dst_end) { - return ENOBUFS; - } + return SUCCESS; +fail: + /* On failure, set the buffer to the empty string for safety */ + *path = '\0'; + return ret; } int make_path_absolute(char *absolute_path, size_t absolute_path_size, const char *path) diff --git a/path_utils/path_utils.h b/path_utils/path_utils.h index 2fd567c..17ed1f4 100644 --- a/path_utils/path_utils.h +++ b/path_utils/path_utils.h @@ -198,7 +198,8 @@ bool is_absolute_path(const char *path); * @param[in] tail The second component of the path * * @return \c SUCCESS if successful, non-zero error code otherwise. - * \li \c ENOBUFS If the buffer space is too small + * \li \c ENOBUFS If the buffer space is too small. In this case, + * path will be set to an empty string. */ int path_concat(char *path, size_t path_size, const char *head, const char *tail); diff --git a/path_utils/path_utils_ut.c b/path_utils/path_utils_ut.c index 34462cf..aadd89b 100644 --- a/path_utils/path_utils_ut.c +++ b/path_utils/path_utils_ut.c @@ -252,24 +252,35 @@ START_TEST(test_path_concat_neg) char small[3]; char small2[5]; char small3[7]; - char p2[9]; + char p2[10]; /* these two test different conditions */ /* Test if head is longer than the buffer */ fail_unless(path_concat(small, 3, "/foo", "bar") == ENOBUFS); + /* On ENOBUFS, path should be empty */ + fail_unless_str_equal(small, ""); /* Test if head is the same length as the buffer */ fail_unless(path_concat(small2, 5, "/foo", "bar") == ENOBUFS); + /* On ENOBUFS, path should be empty */ + fail_unless_str_equal(small2, ""); /* Test if head+tail is the longer than the buffer */ fail_unless(path_concat(small3, 7, "/foo", "bar") == ENOBUFS); + /* On ENOBUFS, path should be empty */ + fail_unless_str_equal(small3, ""); /* off-by-one */ - p2[8] = 1; /* Check whether we've written too far */ + /* Fill with garbage data for now */ + memset(p2, 'Z', 9); + p2[9] = '\0'; + fail_unless(path_concat(p2, 8, "/foo", "bar") == ENOBUFS); - fail_unless(p2[8] == 1); /* This should be untouched */ - fail_unless_str_equal(p2, "/foo/ba"); + /* Make sure we don't write past the end of the buffer */ + fail_unless(p2[8] == 'Z', "Got [%d]", p2[8]); + /* On ENOBUFS, path should be empty */ + fail_unless_str_equal(p2, ""); } END_TEST |