On Mon, 20 Sep 2010 at 22:43:58 +0200, Tamas TEVESZ wrote:
>
> so far only wings is done (mostly), no ill effects seen in a couple of
> hours of use.
Just a couple of trivial things, while trying to digest such a delicate
patch.
> --- a/WINGs/findfile.c
> +++ b/WINGs/findfile.c
> @@ -84,9 +84,8 @@ char *wexpandpath(char *path)
> path++;
> if (*path == '/' || *path == 0) {
> home = wgethomedir();
> - if (strlen(home) > PATH_MAX)
> + if (strlcpy(buffer, home, sizeof(buffer)) >=
> sizeof(buffer))
> goto error;
> - strcat(buffer, home);
> } else {
> int j;
> j = 0;
> @@ -98,9 +97,8 @@ char *wexpandpath(char *path)
> path++;
> }
> home = getuserhomedir(buffer2);
> - if (!home || strlen(home) > PATH_MAX)
> + if (!home || strlcat(buffer, home, sizeof(buffer)) >=
> sizeof(buffer))
> goto error;
> - strcat(buffer, home);
I was wondering why you replaced the first strcat() with strlcpy() while
the second with strlcat().
> /* return address of next char != tok or end of string whichever comes first
> */
> @@ -251,9 +253,16 @@ char *wfindfile(char *paths, char *file)
> path = wmalloc(len + flen + 2);
> path = memcpy(path, tmp, len);
> path[len] = 0;
> - if (path[len - 1] != '/')
> - strcat(path, "/");
> - strcat(path, file);
> + if (path[len - 1] != '/') {
> + if (strlcat(path, "/", len + flen + 2) >= len + flen +
> 2) {
> + wfree(path);
> + return NULL;
> + }
> + }
> + if (strlcat(path, file, len + flen + 2) >= len + flen + 2) {
> + wfree(path);
> + return NULL;
> + }
> fullpath = wexpandpath(path);
> wfree(path);
> if (fullpath) {
> @@ -301,8 +310,11 @@ char *wfindfileinlist(char **path_list, char *file)
> path = wmalloc(len + flen + 2);
> path = memcpy(path, path_list[i], len);
> path[len] = 0;
> - strcat(path, "/");
> - strcat(path, file);
> + if (strlcat(path, "/", len + flen + 2) >= len + flen + 2 ||
> + strlcat(path, file, len + flen + 2) >= len + flen + 2) {
> + wfree(path);
> + return NULL;
> + }
> /* expand tilde */
> fullpath = wexpandpath(path);
> wfree(path);
> @@ -358,8 +370,11 @@ char *wfindfileinarray(WMPropList * array, char *file)
> path = wmalloc(len + flen + 2);
> path = memcpy(path, p, len);
> path[len] = 0;
> - strcat(path, "/");
> - strcat(path, file);
> + if (strlcat(path, "/", len + flen + 2) >= len + flen + 2 ||
> + strlcat(path, file, len + flen + 2) >= len + flen + 2) {
> + wfree(path);
> + return NULL;
> + }
Please put all these multiple "wfree(path); return NULL;" going into one
'goto error', here and in other places in the patch too. You will also
save the { } for the if's, as they become one-liners.
> +static void normalizePath(char *s)
> +{
> + int i, j, found;
> +
> + found = 0;
> + for(i = 0; s[i]; !found && i++) {
'for' is not a function, so for (...)
> + found = 0;
> + if (s[i] == '/' && s[i+1] == '/') {
> + int nslash = 1;
> + found = 1;
> + i++;
> + while(s[i+nslash] == '/')
the same for 'while'
> + nslash++;
> + for(j = 0; s[i+j+nslash]; j++)
again
> + s[i+j] = s[i+j+nslash];
> + s[i+j] = '\0';
> + }
> + }
> + if (i > 1 && s[--i] == '/')
> + s[i] = '\0';
> +}
--
To unsubscribe, send mail to [email protected].
