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. > I would change it to FILE_SHARE_READ|FILE_SHARE_WRITE and remove this code. > > Paolo > 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?