[Box Backup-dev] Reviewing code -- help!

Chris Wilson boxbackup-dev@fluffy.co.uk
Fri, 9 Dec 2005 15:58:54 +0000 (GMT)


Hi Ben,

> OK then... I hope I'm not being too picky here... basically it looks 
> really good and I just want to have a really high quality release.

Thanks, I want the same too. I appreciate your taking the time to review 
my work.

> config.guess, config.sub -- is this necessary?

If we want to cross-compile (native win32 build using cygwin or mingw) 
then unfortunately it appears that we do need these files. Perhaps Martin 
can find a way to automatically copy them from the user's automake 
installation.

> lib/win32/win32test.cpp
> 	-- wrong place, create test/win32

I'm not sure it fits in with your test framework, are you sure?

> lib/win32/win32_build_on_linux_using_mingw.txt
> 	-- move to docs/backup ?

Ok, done.

> lib/win32/win32.bat
> 	-- needs to move to bin/bbackupd/win32

Are you sure? As far as I can tell, this batch file generates the 
autogenerated code on Win32, it's not a way to start bbackupd and doesn't 
need to be distributed. lib/win32 is probably the wrong place, but 
bin/bbackupd/win32 doesn't look right either. Perhaps the root directory 
along with the .vcproj files?

> lib/win32/WinNamedPipeStream.cpp
> 	-- WinNamedPipeStream::Accept(), Connect() doesn't use Name argument.
>
> 	-- define L"\\\\.\\pipe\\boxbackup" in BoxPortsAndFiles.h?

OK, will do.

> 	-- WinNamedPipeStream::GetPeerCredentials() doesn't need any of the 
> UNIX code

And it's commented out (I think). Will probably delete it soon.

> lib/server/SocketStream.h
> 	-- mods belong in Socket.h

SocketStream.h doesn't currently include Socket.h, but would have to in 
order to get the typedef for tOSSocketHandle. Is that OK? Also, it feels 
like action at a distance.

> bin/bbackupd/ServiceBackupDaemon.*
> 	-- rename to Win32ServiceBackupDaemon.* ? or move to bbackupd.cpp?

How about Win32BackupService?

> bin/bbackupd/bbwinservice.*
> 	-- rename to Win32bbackupdService.*

How about Win32ServiceFunctions?

By the way, it compiles and runs on Win32 using MinGW. I'm just working 
out some stability issues at the moment. :-)

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 |