Johannes Schindelin [off-list ref] writes:
+int remove_dir_recursively(char *path, int len, int only_empty)
+{
...
+ namlen = strlen(e->d_name);
+ if (len + namlen > PATH_MAX ||
+ !memcpy(path + len, e->d_name, namlen) ||
+ (path[len + namlen] = '\0') ||
+ lstat(path, &st))
+ ; /* fall thru */
+ else if (S_ISDIR(st.st_mode)) {
+ if (!remove_dir_recursively(path, len + namlen,
+ only_empty))
+ continue; /* happy */
+ } else if (!only_empty &&
+ len + namlen + 1 < PATH_MAX &&
+ !unlink(path))
+ continue; /* happy, too */
+
+ /* path too long, stat fails, or non-directory still exists */
+ ret = -1;
+ break;
Is it only me who finds the first if () condition way too
convoluted and needs to read three times to convince oneself
that it is doing a sane thing?
Please, especially...
* For $DEITY's sake, memcpy() returns pointer to dst which you
know is not NULL. so !memcpy() is always false here, which
might be _convenient_ for you and the compiler but not for
a human reader of the code who needs to blink twice wondering
if you meant !memcmp().
* Same for (path[] = '\0'), wondering if it is misspelled
(path[] == '\0').
Hi,
On Fri, 28 Sep 2007, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
+int remove_dir_recursively(char *path, int len, int only_empty)
+{
...
+ namlen = strlen(e->d_name);
+ if (len + namlen > PATH_MAX ||
+ !memcpy(path + len, e->d_name, namlen) ||
+ (path[len + namlen] = '\0') ||
+ lstat(path, &st))
+ ; /* fall thru */
+ else if (S_ISDIR(st.st_mode)) {
+ if (!remove_dir_recursively(path, len + namlen,
+ only_empty))
+ continue; /* happy */
+ } else if (!only_empty &&
+ len + namlen + 1 < PATH_MAX &&
+ !unlink(path))
+ continue; /* happy, too */
+
+ /* path too long, stat fails, or non-directory still exists */
+ ret = -1;
+ break;
Is it only me who finds the first if () condition way too
convoluted and needs to read three times to convince oneself
that it is doing a sane thing?
Please, especially...
* For $DEITY's sake, memcpy() returns pointer to dst which you
know is not NULL. so !memcpy() is always false here, which
might be _convenient_ for you and the compiler but not for
a human reader of the code who needs to blink twice wondering
if you meant !memcmp().
* Same for (path[] = '\0'), wondering if it is misspelled
(path[] == '\0').
Okay, will fix (with an evil goto).
BTW it just hit me that this magic reliance on a buffer of size PATH_MAX
is not good at all. It even hit _me_ while developing that series.
So I'll change that to a strbuf, too. (Which will fix the convoluted
logic quite some, incidentally.)
Ciao,
Dscho