Re: [PATCH 2/2] mingw: fix mingw_open_append to work with named pipes
- From
- Jeff Hostetler <git@jeffhostetler.com>
- Date
- Sep 10, 2018, 15:44 UTC
- Message-ID
- <0d38ec8e-3f4b-c0fb-ba6f-e2cef39e4db4@jeffhostetler.com>
- In-Reply-To
- <f207bc28-a303-5d63-e9f4-da8e4d466bd5@kdbg.org>
On 9/8/2018 2:31 PM, Johannes Sixt wrote:
Show 40 quoted lines
> Am 08.09.2018 um 11:26 schrieb Johannes Sixt: >> Am 07.09.2018 um 20:19 schrieb Jeff Hostetler via GitGitGadget: >>> diff --git a/compat/mingw.c b/compat/mingw.c >>> index 858ca14a57..ef03bbe5d2 100644 >>> --- a/compat/mingw.c >>> +++ b/compat/mingw.c >>> @@ -355,7 +355,7 @@ static int mingw_open_append(wchar_t const >>> *wfilename, int oflags, ...) >>> * FILE_SHARE_WRITE is required to permit child processes >>> * to append to the file. >>> */ >>> - handle = CreateFileW(wfilename, FILE_APPEND_DATA, >>> + handle = CreateFileW(wfilename, FILE_WRITE_DATA | FILE_APPEND_DATA, >>> FILE_SHARE_WRITE | FILE_SHARE_READ, >>> NULL, create, FILE_ATTRIBUTE_NORMAL, NULL); >>> if (handle == INVALID_HANDLE_VALUE) >>> >> >> I did not go with this version because the documentation >> https://docs.microsoft.com/en-us/windows/desktop/fileio/file-access-rights-constants >> says: >> >> FILE_APPEND_DATA: For a file object, the right to append data to the >> file. (For local files, write operations will not overwrite existing >> data if this flag is specified without FILE_WRITE_DATA.) [...] >> >> which could be interpreted as: Only if FILE_WRITE_DATA is not set, we >> have the guarantee that existing data in local files is not >> overwritten, i.e., new data is appended atomically. >> >> Is this interpretation too narrow and we do get atomicity even when >> FILE_WRITE_DATA is set? > > Here is are some comments on stackoverflow which let me think that > FILE_APPEND_DATA with FILE_WRITE_DATA is no longer atomic: > > https://stackoverflow.com/questions/20093571/difference-between-file-write-data-and-file-append-data#comment29995346_20108249 > > > -- Hannes
Yeah, this whole thing is a little under-documented for my tastes. Let's leave it as you have it. I'll re-roll with a fix to route named pipes to the existing _wopen() code.
thanks Jeff