On Mon, Feb 28, 2022 at 4:27 PM Jeff Hostetler <git@xxxxxxxxxxxxxxxxx> 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.
