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);
--
Takashi Yano <takashi.y...@nifty.ne.jp>
