[PATCH 3/7] server: Fail to set delete disposition on a non-empty directory.

[Alexandre Julliard]

Dmitry Timoshkov <dmitry at baikal.ru> writes:

> Alexandre Julliard <julliard at winehq.org> wrote:
>
>> >> >> >> > +static int is_directory_empty( struct fd *fd )
>> >> >> >> > +{
>> >> >> >> > +    DIR *dir;
>> >> >> >> > +    int count = 0;
>> >> >> >> > +
>> >> >> >> > +    if ((dir = fdopendir( fd->unix_fd )))
>> >> >> >> > +    {
>> >> >> >> > +        while (readdir( dir ) != NULL && count <= 2)
>> >> >> >> > +            count++;
>> >> >> >> > +
>> >> >> >> > +        closedir( dir );
>> >> >> >> > +    }
>> >> >> >> 
>> >> >> >> This won't work, closedir() is going to close the file descriptor.
>> >> >> >
>> >> >> > Would it be acceptable to duplicate the fd before the check?
>> >> >> 
>> >> >> That's what you'd have to do, yes. But honestly I'm not sure that this
>> >> >> check is a good idea in the first place. I wonder if we shouldn't remove
>> >> >> the directory right away instead.
>> >> >
>> >> > Setting a disposition is not supposed to remove a directory or a file, just
>> >> > mark it for the removal until the last handle to it gets closed, or do you
>> >> > mean something else?
>> >> 
>> >> I know it's not supposed to, but I'm not sure it's a good enough reason
>> >> to add this check and make RemoveDirectory less reliable. Do you have an
>> >> app that depends on the directory still existing?
>> >
>> > It's the task scheduler service that suffers from this.
>> 
>> Where does it depend on the directory still existing?
>
> The scheduler service opens a directory for monitoring, and if that
> dir gets removed the things go haywire. I have an installer that calls
> RemoveDirectory() for the Tasks and Fonts directories, probably that
> happens inadvertently but still.

It sounds like the scheduler service should be fixed. The directory
could also get removed from outside of Wine, it should be able to cope
with that.

-- 
Alexandre Julliard
julliard at winehq.org