Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
> On Fri, 28 Sep 2007, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@xxxxxx> 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').
>
> Okay, will fix (with an evil goto).
Uh, C has a comma operator. And inverting the first condition makes
for a nicer flow, no goto.
namlen = strlen(e->d_name);
if (len + namlen <= PATH_MAX
&& (memcpy(path + len, e->d_name, namlen),
path[len + namlen] = '\0',
lstat(path, &st) == 0)
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;
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
