[PATCH v3] server: Fail if non-empty directory marked for deletion.

[Chip Davis]

May 28, 2020 7:44 PM, "Daniel Lehman" <dlehman25 at gmail.com> wrote:

>> + empty = 1;
>> + while (empty && (de = readdir( dir )))
>> + {
>> + if (!strcmp( de->d_name, "." ) || !strcmp( de->d_name, ".." )) continue;
>> + empty = 0;
>> 
>> You could probably break from the loop after this point, since you found what you were looking for.
> 
> it does break the loop. i originally had an explicit break, but
> changed it to break when empty is zeroed. i can switch it back if
> preferred

Ah, I see now. Sorry about that. It should be fine as is.

> 
>> + }
>> + closedir( dir );
>> + close( fd );
>> 
>> This close(2) call shouldn't be necessary, since closedir(3) does that for you. It's the reason I
>> suggested dup(2)'ing the fd in the first place. Leaving this in will set errno to EBADF, which may
>> not be desirable.
> 
> i had considered that, but the documentation suggests that using a
> file descriptor is implementation-dependent. i know that's the case
> for linux, but not sure about the others. from docs, it looks like
> freebsd, macosx, and android are the same. if that covers it, i'll
> remove the close

Let's see what POSIX has to say:

  Upon calling closedir() the file descriptor shall be closed.[1]

Most modern systems should conform to POSIX. I think you should be OK removing the close(2) call.

[1]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/fdopendir.html#

Chip