Il 21/09/2012 14:17, Jeff Cody ha scritto:
> On 09/21/2012 04:43 AM, Paolo Bonzini wrote:
>> Il 21/09/2012 10:33, Kevin Wolf ha scritto:
>>>>> + /* could not reopen the file handle, so fall back to opening
>>>>> + * new file (CreateFile) */
>>>>> + if (raw_s->hfile == INVALID_HANDLE_VALUE) {
>>>>> + raw_s->hfile = CreateFile(state->bs->filename, access_flags,
>>>>> + FILE_SHARE_READ, NULL, OPEN_EXISTING,
>>>>> + overlapped, NULL);
>>>>> + if (raw_s->hfile == INVALID_HANDLE_VALUE) {
>>>>> + /* this could happen because the access_flags requested are
>>>>> + * incompatible with the existing share mode of s->hfile,
>>>>> + * so our only option now is to close s->hfile, and try
>>>>> again.
>>>>> + * This could end badly */
>>>>> + CloseHandle(s->hfile);
>>> How common is this case?
>>>
>>> We do have another option, namely not reopen at all and return an error.
>>> Of course, this only makes sense if it doesn't mean that we almost never
>>> succeed.
>>
>> Probably pretty common since we specify FILE_SHARE_READ for the sharing
>> mode, meaning that "subsequent open operations on a file or device are
>> only able to request read access".
>
> Yes, I think this is by far the most common case.
Actually ReOpenFile probably only takes into account _other_ sharing
modes, not the one for hFile, so it may even be unnecessary.
But...
>> I would change it to FILE_SHARE_READ|FILE_SHARE_WRITE and remove this code.
>
> I contemplated doing that, but I wasn't sure if there was any particular
> reason it was originally done with FILE_SHARE_READ only in the first
> place (security, etc..). I was hesitant to override that behaviour as
> the new default under w32. Do we know if this is acceptable / safe?
... let's just make things work the same as in Unix.
Paolo
