[Box Backup-dev] Merges

Chris Wilson boxbackup-dev at fluffy.co.uk
Mon Aug 21 00:39:47 BST 2006


Hi Martin,

On Sun, 20 Aug 2006, Martin Ebourne wrote:

>>    svn diff -r 748:803 \
>>    http://bbdev.fluffy.co.uk/svn/box/chris/merge/bin/bbackupd/BackupDaemon.cpp
>
> 			catch(std::exception &e)
> 			{
> 				::syslog(LOG_ERR, "Internal error while "
> 					"closing command socket after "
> 					"another exception: %s", e.what());
> 			}
> 			catch(...)
> 			{
> 				::syslog(LOG_WARNING,
> 					"Error closing command socket "
> 					"after exception, ignored.");
> 			}
>
> Why is one an error and one a warning?

I was trying to be consistent with other places where we caught 
std::exception, since it indicates an unexpected error rather than a (more 
likely) socket error, and therefore seemed more serious. But I've changed 
it because you asked, and because it's only during cleanup from an 
exception anyway.

> Having been and looked at the code I can see that BoxException already
> implements std::exception. At least the first try/catch in
> RunHelperThread could therefore have the catch(BoxException) block
> removed since the catch(std::exception) block will cover it and do
> exactly the same thing.

OK, done.

> Probably also the one at the end of the same method (though keep the 
> message from the BoxException case to use for std::exception).

I'm not sure about that. An internal error such as running out of memory 
is different to a socket error, so I think we should report them 
differently.

> At line 804 it seems a shame that the standard exceptions can't be
> reported like the box ones. Maybe if we reserved a type/subtype value
> for std::exceptions we could use the same mechanism?

Perhaps, but I'll wait for Ben to say what he wants to do here before I do 
that. In any case, we'd still need two blocks, and as the comments say, 
this part of the code should probably be reviewed for exception safety 
anyway.

> I wonder if you intentionally skipped the one at line 2343?

No, well spotted.

> There were some waitpid changes in this diff that weren't in the
> previous one. Were they accidental?

No, deliberate. Rather than copying the same code yet again, I removed the 
existing duplication. I think the effect is identical.

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 |




More information about the Boxbackup-dev mailing list