[Box Backup-dev] changeset [819]

[Chris Wilson]

Hi Martin and Ben,

>> >  Perhaps for future work you could try and use the minimum possible 
>> >  #ifdefs. For example, because you no longer have the housekeeping 
>> >  process, you ifdef out everything related. However, you could have 
>> >  left it all in, and just ifdefs out the code which does the actual 
>> >  connection and talking, and level everything else in place. Might 
>> >  not have made all that difference though.
>> 
>>  OK, will do. Would you like me to remove them?
>
> I certainly would. Only passed the code review because I assumed you 
> needed to disable it.

I have a question about what to do with the #ifdefs in 
bin/bbstored/BackupCommands.cpp (changeset 819, 
http://bbdev.fluffy.co.uk/trac/changeset/819).

I can remove the first one without problems, and use the same temporary 
file name on Win32 and other platforms.

However, the second one, which deletes the temporary files, is tricky. 
Under Win32, we simply cannot delete the last file, which is returned to 
the caller to be streamed to the client, because it's still open at the 
time, and Windows won't allow us to delete it.

The caller can't delete it either, because it doesn't know whether that 
file is a temporary file, or the last one in the patch set, which is the 
actual current revision. Only this function, 
BackupProtocolServerGetFile::DoCommand, knows whether it should be deleted 
or not.

At the moment, I don't delete the files at all on Windows, which is 
obviously a bug, and requires an #ifdef.

There is a possible clean solution, which is to use the Windows flag 
FILE_FLAG_DELETE_ON_CLOSE when opening the file. To avoid the #ifdef, I 
think I would have to invent a new flag to pass to the FileStream 
constructor, which does nothing (define to zero) on other platforms, but 
activates this Windows flag in openfile() on Win32.

Does this seem like a sensible solution? And what should that flag be 
called? O_DELETE_ON_CLOSE_WIN32?

Does anyone have any other ideas?

Cheers, Chris.
-- 
_ ___ __     _
  / __/ / ,__(_)_  | Chris Wilson <0000 at qwirx.com> - Cambs UK |
/ (_/ ,\/ _/ /_ \ | Security/C/C++/Java/Perl/SQL/HTML Developer |
\ _/_/_/_//_/___/ | We are GNU-free your mind-and your software |