git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/2] mingw: fix mingw_open_append to work with named pipes

From
JHJeff 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

Previous: Johannes SixtNext: Junio C Hamano
Message 8 of 21 in “Fixup for js/mingw-o-append”
  1. 0/2 Fixup for js/mingw-o-appendJeff Hostetler via GitGitGadget, Sep 7, 2018
  2. 1/2 t0051: test GIT_TRACE to a windows named pipeJeff Hostetler via GitGitGadget, Sep 7, 2018
  3. Sebastian SchuberthSep 9, 2018
  4. Jeff HostetlerSep 10, 2018
  5. 2/2 mingw: fix mingw_open_append to work with named pipesJeff Hostetler via GitGitGadget, Sep 7, 2018
  6. Johannes SixtSep 8, 2018
  7. Johannes SixtSep 8, 2018
  8. Jeff HostetlerSep 10, 2018
  9. Junio C HamanoSep 10, 2018
  10. Jeff HostetlerSep 10, 2018
  11. Jeff HostetlerSep 7, 2018
  12. 0/2 Fixup for js/mingw-o-appendJeff Hostetler via GitGitGadget, Sep 10, 2018
  13. 1/2 t0051: test GIT_TRACE to a windows named pipeJeff Hostetler via GitGitGadget, Sep 10, 2018
  14. 2/2 mingw: fix mingw_open_append to work with named pipesJeff Hostetler via GitGitGadget, Sep 10, 2018
  15. Johannes SixtSep 10, 2018
  16. Jeff HostetlerSep 10, 2018
  17. Junio C HamanoSep 10, 2018
  18. Jeff HostetlerSep 11, 2018
  19. 0/2 Fixup for js/mingw-o-appendJeff Hostetler via GitGitGadget, Sep 11, 2018
  20. 1/2 t0051: test GIT_TRACE to a windows named pipeJeff Hostetler via GitGitGadget, Sep 11, 2018
  21. 2/2 mingw: fix mingw_open_append to work with named pipesJeff Hostetler via GitGitGadget, Sep 11, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.