[PATCH 3/7] server: Fail to set delete disposition on a non-empty directory.
Dmitry Timoshkov
dmitry at baikal.ru
Thu Nov 8 09:29:50 CST 2018
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.
--
Dmitry.
More information about the wine-devel
mailing list