[Box Backup-dev] Merges

Martin Ebourne boxbackup-dev at fluffy.co.uk
Sat Aug 19 23:18:59 BST 2006


Except where noted, these are all good to merge:

> Please review 738:739 for merge.
> Please review 739:740 for merge.
> Please review 740:741 for merge.
> Please review 742:743 for merge.
> Please review 745:746 for merge.
> Please review 746:747 for merge.
> Please review 747:748 for merge.
> Please review 748:749 for merge.

1.
--- bin/bbackupd/BackupDaemon.cpp       (revision 748)
+++ bin/bbackupd/BackupDaemon.cpp       (revision 749)
@@ -263,16 +263,36 @@
 void BackupDaemon::RunHelperThread(void)
 {
        mpCommandSocketInfo = new CommandSocketInfo;
-       this->mReceivedCommandConn = false;

Why?

2. If you've got a catch(...) and you're not propagating the error you
should at least catch(std::exception&) before it and report that
with .what(). This will catch a lot of exceptions (eg. out of memory)
that would otherwise go reported as "unknown error". I see too much
software give unknown error and it's quite hateful. This is also true of
the catch at the bottom of this function too, which you haven't changed
here.

3. Could you not use IsTerminateWanted() rather than break to get of the
loop?

> Please review 749:750 for merge.
> Please review 754:755 for merge.
> Please review 755:756 for merge.

1. I'm guessing Ben might have something to say about the naming of the
new struct.

2. This adds new options 'T' to list and 'A' to compare, but no code to
handle them.

> Please review 757:758
> Please consider 766:767 for merge.

1. What if rDir to MakeFullPath is empty? If that's impossible it should
be documented as a precondition.

2. Why are you changing copy construction of locals to assignment? Copy
construction is better.

As far as I can see from my inbox this is the full list of outstanding
merges. If I've missed any please correct me. Again, sorry for the
delay.

Cheers,

Martin.




More information about the Boxbackup-dev mailing list