On Wed, Oct 23, 2024 at 06:17:15PM +0200, Kristoffer Haugsbakk wrote:
> On Wed, Oct 23, 2024, at 17:05, Patrick Steinhardt wrote:
> > While we have the same issue when calling open(3p) with `O_CREAT`,
>
> s/O_CREAT/O_CREATE/ ?
Yeah, you'd think that, but no, it's really `O_CREAT`.
> > +static int mingw_open_existing(const wchar_t *filename, int oflags, ...)
> > +{
> > + SECURITY_ATTRIBUTES security_attributes = {
> > + .nLength = sizeof(security_attributes),
> > + .bInheritHandle = !(oflags & O_NOINHERIT),
>
> `clang-format` thinks that these two lines should be indented with tabs
> instead (so two tabs in total).
>
> (Ubuntu clang-format version 14.0.0-1ubuntu1.1)
And they indeed should be.
> > + };
> > + HANDLE handle;
> > + int access;
> > + int fd;
> > +
> > + /* We only support basic flags. */
> > + if (oflags & ~(O_ACCMODE | O_NOINHERIT))
> > + return errno = ENOSYS, -1;
>
> This use of the comma operator is maybe an idiom to save space and avoid
> a brace around the `if`? This pattern is already in use in
> `mingw_open_append`. I see in `mingw.h` that it uses:
>
> ```
> static inline int symlink(const char *oldpath UNUSED, const char *newpath UNUSED)
> { errno = ENOSYS; return -1; }
> ```
I basically copied this from other code, but don't care strongly about
it either way.
> > + if (oflags & O_RDWR)
> > + access = GENERIC_READ | GENERIC_WRITE;
> > + else if (oflags & O_WRONLY)
> > + access = GENERIC_WRITE;
> > + else
> > + access = GENERIC_READ;
> > +
> > + handle = CreateFileW(filename, access,
> > + FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE,
> > + &security_attributes, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
>
> `clang-format` says that these two lines are too long.
Might be. But the `FILE_SHARE_*` line cannot really be any shorter
without hurting readability. And the second line just follows the first
line then.
Patrick
