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

[Ben Summers]

On 9 Dec 2005, at 15:58, Chris Wilson wrote:

>> 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.

Not terribly bothered if it's necessary, although maybe the  
infrastructure/makedistribution.pl script could do the copying when  
it's all wrapped up for release?

>
>> lib/win32/win32test.cpp
>> 	-- wrong place, create test/win32
>
> I'm not sure it fits in with your test framework, are you sure?

Can it not be made to do so?

>
>> 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?

Oooops, you're right.

I suppose infrastructure/win32/ might be the logical place for it,  
although it's rather inconvenient. And the VS project files would be  
nicer there too... but on a pragmatic note, sticking it all in the  
root is going to be perfect for now.

>> 	-- WinNamedPipeStream::GetPeerCredentials() doesn't need any of  
>> the UNIX code
>
> And it's commented out (I think). Will probably delete it soon.

Since that file is only compiled on Win32, it's unnecessary cruft.

>
>> 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.

There's arguments either way. Let's leave it as it is, because  
SocketStream is the abstraction of a socket for this project anyway.

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

Far better!

>
>> bin/bbackupd/bbwinservice.*
>> 	-- rename to Win32bbackupdService.*
>
> How about Win32ServiceFunctions?

Lovely!

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

Great news. I'm really glad we're progressing so well towards a  
unified tree and then a release from which we can move forward cleanly.

Ben