[Box Backup-dev] Merges

[Martin Ebourne]

On Sun, 2006-08-20 at 01:47 +0100, Chris Wilson wrote:
> Ok, I'm no expert on exceptions or C++, and I've never seen a 
> std::exception in the wild,

std::exception is the interface that all C++ standardised exceptions
implement. Both the run time code and the libraries are capable of
throwing objects implementing std::exception. Here's a good breakdown of
the class hierarchy as provided by the ISO C++ 99 standard:

http://gcc.gnu.org/onlinedocs/libstdc++/latest-doxygen/classstd_1_1exception.html

Some of those can happen virtually anywhere - eg. bad_alloc is used for
out of memory and can be thrown any time new is called, which makes it
pretty likely.

If you're using catch to report an error in C++ you should always catch
std::exception and report that otherwise you're missing a whole load of
exception types. A lot of recent apps also implement std::exception for
their own exception types.

Only time to use catch(...) and not catch(std::exception) as well is if
you're doing silent cleanup and then rethrowing.

>  but I found quite a few other places in 
> BackupDaemon that looked like they could use the same treatment, so I 
> fixed them all.

Excellent, good thinking.

> I don't like using break, but the alternative seemed to be to set a flag 
> and call continue, which looks worse to me. At least "break;" with a good 
> comment has an intuitive meaning, whereas using "continue" to kill the 
> thread is counter-intuitive, isn't it?

Ok, that's fine then.

> Please review for merge:
> 
>    svn diff -r 748:798 \
>    http://bbdev.fluffy.co.uk/svn/box/chris/merge/bin/bbackupd/BackupDaemon.cpp

Can you go through this and check all your added catch blocks. At least
one of them the catch(std::exception) doesn't do the same as the
catch(...).

> Fixed both. Please review for merge:
> 
>    svn diff -r 766:800 \
>    http://bbdev.fluffy.co.uk/svn/box/chris/general/bin/bbackupd/BackupClientDirectoryRecord.cpp

Yep, that's ok now.

> By the way, why is copy construction better? More efficient?

Ben answered that in detail.

> > 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.
> 
> Thanks again! OK if I send you some more patches for review this weekend?

Sure, just keep sending them as and when. I'll review them when I can
grab some time. Can't guarantee it'll be this weekend though.

Cheers,

Martin.