Re: [PATCH 1/2] t/helper/test-chmtime: update mingw to support chmtime on directories

[Tao Klerks <tao@klerks.biz>]

On Mon, Feb 28, 2022 at 4:27 PM Jeff Hostetler <git@jeffhostetler.com> wrote:
>
>
>
> On 2/28/22 4:40 AM, Tao Klerks via GitGitGadget wrote:

> > +     HANDLE osfilehandle;
>
> I'd be more comfortable initializing this variable to
> INVALID_HANDLE_VALUE.
>

Makes sense, thanks. (but less relevant after switch to CreateFileW)

> > +             fh = 0;
>
> and here initializing fh = -1.

Makes sense, thanks. (but overshadowed by switch to CreateFileW)

> > +             osfilehandle = CreateFileW(wfilename,
> > +                                        0x100 /*FILE_WRITE_ATTRIBUTES*/,
> > +                                        0 /*FileShare.None*/,
>
> Is there a reason that you're not using the #define's here?
> I assume you ran into a header file problem or something, but
> there are other symbols used nearby, so I'm not sure.

I couldn't find these, and am a complete C APIs n00b - I have no idea
whether or how to add them. I figured commenting on their meaning
is the simplest safest thing to do locally?

> > +     if (fh)
> > +             close(fh);
> > +     else if (osfilehandle)
> > +             CloseHandle(osfilehandle);
>
> And then this becomes:
>
>         if (fh != -1)
>                 close(fh)
>         else if (osfilehandle != INVALID_HANDLE_VALUE)
>                 Closehandle(osfilehandle);
>
> Which I think makes it more clear that we'll properly close the handle.

Perfect, thx.

>
> >
> >   revert_attrs:
> >       if (attrs != INVALID_FILE_ATTRIBUTES &&
> >
>
> An alternative solution would be to replace the _wopen() with
> a call to CreateFileW() without the backup flag for non-directories
> and just convert the whole routine to use HANDLE's rather fd's.
>

I was at first scared of making changes to things I don't fully
understand, but looking at the existing use of GetFileAttributesW
I don't think that makes much sense. This is cleaner/simpler
even if it is a bigger change, and consistent with other things.