[Box Backup] Code in SVN -- review notes

[Ben Summers]

I've taken a look at the code checked in recently, notes below. Does  
anyone else feel like reviewing the code?

Generally, it's almost ready to merge in, and some handy stuff to  
have. But the Win32 ports needs more attention as it's rather more  
complex. Especially wrt to the build environment.

Thanks for all the efforts of the contributors. I notice none of you  
have bothered adding a decent CONTRIBUTORS file, this must be rectified.

Ben



martin/solaris
==============
lib/server/Protocol.cpp -- can we avoid those extra memcpys? Is it  
actually causing a problem with some processor without the ability to  
do non-word aligned loads? Can we make it conditional on being run on  
one of those processors using PLATFORM_ALIGN_INT?

Otherwise all good.


martin/ppcfixes
===============
All good.


martin/xattr
============
xattr is also available on Darwin, so the name of the test is  
probably misleading.

My worries is that there may be quite a bit of xattr data, and the  
way BackupClientFileAttributes are used assumes that they are  
relatively small. I think they would be better represented as a  
stream. But then, we need to write the code to support multiple streams.

Under Darwin, it'll pick up the resource fork as an xattr. This could  
be quite large!

A minor point, but in the format of the structure, you would have to  
store a zero for the xattr size if it was extended afterwards. Also  
if it was stored here, perhaps I would prefer the ability to stack  
multiple attribute records, so you'd get a standard UNIX one and  
another xattr one?

I would be happy for this to be committed, as long as there was a  
commitment (!) to revisit this code when multiple streams are  
available in backup files.

(I'm quite keen on getting xattr's done properly. Another project of  
mine [ http://www.fluffy.co.uk/spotmeta/ ] uses them quite extensively!)


martin/autoconf
===============
You appear to have removed my code which means someone has to take  
positive action to working with old versions of OpenSSL. I'd prefer  
that stayed -- it's a nasty hack to do the stuff I need with older  
OpenSSLs.

infrastructure/BoxPlatform.pm - license slipped in!

I think it looks OK (reading the diffs carefully) but I'm not sure  
how to actually compile it. We need to try it out on "all" platforms.

Also, how do we go about making a distribution? It'd be nice to have  
something in docs/.


chromi/diffopt
==============
static int sMaximumDiffTime = 600;             // maximum time to  
spend diffing
     -- not sure whether this is good or not?

Style: A tendency to place {'s on the same line as ifs and whiles.  
And "} else {"...

Is docs/backup/encrypt_rsync.txt changed, or just renamed? Use svn  
rename first anyway.

Looks good, these are very minor points. But I would appreciate  
consistent style.


nick/win
========
To be honest, this scares me. Also, the DOS line endings screw up the  
diffs. Something to look at in more detail another day, I'm afraid.  
If someone would like to commit one with the line endings The  
Invisible Sky Giant intended us to use, that would be lovely.