[Box Backup-dev] 0.11rc1 win32 code review

Charles Lecklider boxbackup-dev at fluffy.co.uk
Sun Jan 27 22:00:42 GMT 2008


I had a few spare minutes today so I thought I'd take a quick look 
through the code for rc1. I've not looked through everything, but what I 
did seems to have far fewer bits of Win32-specific stuff scattered 
through it, and that can only be a good thing.

If there are any Win32 parts I've not listed that you think are worth a 
look let me know and I'll try to find some more time later this week.

-C



lib\backupclient\BackupClientCryptoKeys.cpp:

101: Use SecureZeroMemory() instead of memset(). See also 
lib\common\ZeroStream.cpp. I think OpenSSL has its own secure version of 
memset() which would make cross-platform stuff easier.


lib\win32\emu.cpp:

FormatMessage() needs FORMAT_MESSAGE_IGNORE_INSERTS set.


lib\server\WinNamedPipeStream.cpp:

130: Flags say BYTE mode pipe, comments say MESSAGE mode. Which is it 
supposed to be?

150: If a pipe is opened with FILE_FLAG_OVERLAPPED then all operations 
should be overlapped - including ConnectNamedPipe(). Use 
GetOverlappedResult() to, erm, get the result ;-)

215: Consider using WaitNamedPipe() with a sensible timeout, or don't 
use NMPWAIT_USE_DEFAULT_WAIT when creating the pipe.

167/398: ResetEvent() isn't needed if the event isn't a manual reset type.

472: See 150.

527: Not a good idea to rely on CancelIo() - it only cancels things on 
the thread that started them.




More information about the Boxbackup-dev mailing list