[Box Backup-dev] Merges

Martin Ebourne boxbackup-dev at fluffy.co.uk
Sun Aug 20 23:38:10 BST 2006


On Sun, 2006-08-20 at 11:13 +0100, Chris Wilson wrote:
> OK, fixed, I think. Please review for merge:
> 
>    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?

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. Probably also the one at the end of the same
method (though keep the message from the BoxException case to use for
std::exception).

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?

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

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

-               // Clean up though
-               if(pid != 0)
-               {
-                       int status = 0;
-                       ::waitpid(pid, &status, 0);
-               }

> Thanks, please review for merge:
> 
>    svn diff -r 755:802
>    http://bbdev.fluffy.co.uk/svn/box/chris/merge/bin/bbackupquery/BackupQueries.cpp

Yes, that's fine.

Cheers,

Martin.




More information about the Boxbackup-dev mailing list