[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