Hi Corinna,
On Tue, 10 Dec 2024 22:10:57 +0900
Takashi Yano wrote:
> On Tue, 10 Dec 2024 14:00:53 +0100
> Corinna Vinschen wrote:
> > Hi Takashi,
> >
> > On Dec 10 21:21, Takashi Yano wrote:
> > > Hi Corinna,
> > >
> > > On Mon, 9 Dec 2024 16:26:53 +0100
> > > Corinna Vinschen wrote:
> > > > On Dec 9 22:57, Takashi Yano wrote:
> > > > > On Mon, 9 Dec 2024 22:44:00 +0900
> > > > > Takashi Yano wrote:
> > > > > > On Mon, 9 Dec 2024 12:11:56 +0100
> > > > > > Corinna Vinschen wrote:
> > > > > > > init_reopen_attr() uses the "open by handle" functionality as in
> > > > > > > the
> > > > > > > Win32 API ReOpenFile(). It only does so if the filesystem
> > > > > > > supports it.
> > > > > > > Samba usually does, so it's not clear to me why
> > > > > > > pc.init_reopen_attr()
> > > > > > > fails for you.
> > > > > >
> > > > > > I didn't mean pc.init_reopen_attr() failed. Just I was no idea
> > > > > > for what handle to be passed.
> > > > > >
> > > > > > > > What handle should I pass to pc.init_reopen_attr()?
> > > > > > >
> > > > > > > You could pass pc.handle(). Is pc.handle() in this scenario NULL,
> > > > > > > perhaps?
> > > > > >
> > > > > > I have tried pc.handle() and suceeded. Thanks for advice!
> > > > >
> > > > > No! pc.handle() sometimes seems to be NULL....
> > > >
> > > > Can you please figure out in which scenario it's NULL? Theoretically
> > > > the function shouldn't even be called in this case.
> > >
> > > This seems to happen when check_file_access() is called from av::setup()
> > > (spawn.cc:1237) called from child_info_spawn::worker() (spawn.cc:358).
> >
> > Huh, yeah, thanks for tracking this down. And this code snippet is
> > actually only called if we *failed* opening the file, so there's no
> > usable handle.
> >
> > Try this:
> >
> >
> > From 23387c343381ba01d02210257e33cf2691611c2d Mon Sep 17 00:00:00 2001
> > From: Corinna Vinschen <cori...@vinschen.de>
> > Date: Tue, 10 Dec 2024 13:55:54 +0100
> > Subject: [PATCH] Cygwin: path_conv: allow NULL handle in init_reopen_attr()
> >
> > init_reopen_attr() doesn't guard against a NULL handle. However,
> > there are scenarios calling functions deliberately with a NULL handle,
> > for instance, av::setup() calling check_file_access() only if opening
> > the file did NOT succeed.
> >
> > So check for a NULL handle in init_reopen_attr() and if so, use the
> > name based approach filling the OBJECT_ATTRIBUTES struct, just as in
> > the has_buggy_reopen() case.
> >
> > Fixes: 4c9d01fdad2a ("* mount.h (class fs_info): Add has_buggy_reopen flag
> > and accessor methods.")
> > Signed-off-by: Corinna Vinschen <cori...@vinschen.de>
> > ---
> > winsup/cygwin/local_includes/path.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/winsup/cygwin/local_includes/path.h
> > b/winsup/cygwin/local_includes/path.h
> > index 3dd21d975abf..2a05cf44d40a 100644
> > --- a/winsup/cygwin/local_includes/path.h
> > +++ b/winsup/cygwin/local_includes/path.h
> > @@ -323,7 +323,7 @@ class path_conv
> > }
> > inline POBJECT_ATTRIBUTES init_reopen_attr (OBJECT_ATTRIBUTES &attr,
> > HANDLE h)
> > {
> > - if (has_buggy_reopen ())
> > + if (!h || has_buggy_reopen ())
> > InitializeObjectAttributes (&attr, get_nt_native_path (),
> > objcaseinsensitive (), NULL, NULL)
> > else
> > --
> > 2.47.0
> >
>
> Thanks! Is your patch better than?
> + if (pc.handle ())
> + pc.init_reopen_attr (attr, pc.handle ());
> + else
> + pc.get_object_attr (attr, sec_none_nih);
I confirmed that both above two patches work.
--
Takashi Yano <takashi.y...@nifty.ne.jp>
