[Merge] lp:~mandel/dbus-cpp/expose_executor into lp:dbus-cpp

Thomas Voß thomas.voss at canonical.com
Thu Jun 5 15:13:59 UTC 2014


Review: Needs Fixing



Diff comments:

> === modified file 'debian/changelog'
> --- debian/changelog	2014-06-04 07:29:10 +0000
> +++ debian/changelog	2014-06-05 14:12:25 +0000
> @@ -1,3 +1,4 @@
> +<<<<<<< TREE
>  dbus-cpp (3.0.0+14.10.20140604-0ubuntu1) utopic; urgency=low
>  
>    [ Jussi Pakkanen ]
> @@ -5,6 +6,14 @@
>  
>   -- Ubuntu daily release <ps-jenkins at lists.canonical.com>  Wed, 04 Jun 2014 07:29:10 +0000
>  
> +=======
> +dbus-cpp (3.1.0-0ubuntu1) UNRELEASED; urgency=medium
> +
> +  * Provide a new make_executor method to allow to pass the io_service.
> +
> + -- Manuel de la Pena <manuel.delapena at canonical.com>  Thu, 05 Jun 2014 13:53:45 +0200
> +
> +>>>>>>> MERGE-SOURCE
>  dbus-cpp (3.0.0+14.10.20140507-0ubuntu1) utopic; urgency=medium
>  
>    [ Thomas Voß ]
> 
> === modified file 'debian/libdbus-cpp3.symbols.32bit'
> --- debian/libdbus-cpp3.symbols.32bit	2014-05-05 08:38:36 +0000
> +++ debian/libdbus-cpp3.symbols.32bit	2014-06-05 14:12:25 +0000
> @@ -1,4 +1,5 @@
>   (c++)"core::dbus::asio::make_executor(std::shared_ptr<core::dbus::Bus> const&)@Base" 2.0.0+14.04.20140310
> + (c++)"core::dbus::asio::make_executor(std::shared_ptr<core::dbus::Bus> const&, boost::asio::io_service &)@Base" 0replaceme
>   (c++)"core::dbus::Bus::access_signal_router()@Base" 2.0.0+14.04.20140310
>   (c++)"core::dbus::Bus::add_match(core::dbus::MatchRule const&)@Base" 2.0.0+14.04.20140310
>   (c++)"core::dbus::Bus::~Bus()@Base" 2.0.0+14.04.20140310
> @@ -200,4 +201,4 @@
>   (c++)"vtable for core::dbus::Fixture at Base" 2.0.0+14.04.20140310
>   (c++)"vtable for core::dbus::PendingCall at Base" 2.0.0+14.04.20140310
>   (c++)"vtable for core::dbus::types::ObjectPath::Errors::InvalidObjectPathStringRepresentation at Base" 2.0.0+14.04.20140310
> -#include "libdbus-cpp3.symbols.coverage"
> \ No newline at end of file
> +#include "libdbus-cpp3.symbols.coverage"
> 
> === modified file 'debian/libdbus-cpp3.symbols.64bit'
> --- debian/libdbus-cpp3.symbols.64bit	2014-05-05 08:38:36 +0000
> +++ debian/libdbus-cpp3.symbols.64bit	2014-06-05 14:12:25 +0000
> @@ -1,4 +1,5 @@
>   (c++)"core::dbus::asio::make_executor(std::shared_ptr<core::dbus::Bus> const&)@Base" 2.0.0+14.04.20140310
> + (c++)"core::dbus::asio::make_executor(std::shared_ptr<core::dbus::Bus> const&, boost::asio::io_service &)@Base" 0replaceme
>   (c++)"core::dbus::Bus::access_signal_router()@Base" 2.0.0+14.04.20140310
>   (c++)"core::dbus::Bus::add_match(core::dbus::MatchRule const&)@Base" 2.0.0+14.04.20140310
>   (c++)"core::dbus::Bus::~Bus()@Base" 2.0.0+14.04.20140310
> @@ -200,4 +201,4 @@
>   (c++)"vtable for core::dbus::Fixture at Base" 2.0.0+14.04.20140310
>   (c++)"vtable for core::dbus::PendingCall at Base" 2.0.0+14.04.20140310
>   (c++)"vtable for core::dbus::types::ObjectPath::Errors::InvalidObjectPathStringRepresentation at Base" 2.0.0+14.04.20140310
> -#include "libdbus-cpp3.symbols.coverage"
> \ No newline at end of file
> +#include "libdbus-cpp3.symbols.coverage"
> 
> === modified file 'include/core/dbus/asio/executor.h'
> --- include/core/dbus/asio/executor.h	2013-11-27 18:57:42 +0000
> +++ include/core/dbus/asio/executor.h	2014-06-05 14:12:25 +0000
> @@ -22,6 +22,17 @@
>  #include <core/dbus/executor.h>
>  #include <core/dbus/visibility.h>
>  
> +namespace boost
> +{
> +namespace asio
> +{
> +

unneeded whitespace

> +class io_service;
> +

unneeded whitespace

> +}
> +

unneeded whitespace

> +}
> +
>  namespace core
>  {
>  namespace dbus
> @@ -29,6 +40,7 @@
>  namespace asio
>  {
>  ORG_FREEDESKTOP_DBUS_DLL_PUBLIC Executor::Ptr make_executor(const Bus::Ptr& bus);
> +ORG_FREEDESKTOP_DBUS_DLL_PUBLIC Executor::Ptr make_executor(const Bus::Ptr& bus, boost::asio::io_service& io);
>  }
>  }
>  }
> 
> === modified file 'src/core/dbus/asio/executor.cpp'
> --- src/core/dbus/asio/executor.cpp	2014-03-05 06:58:15 +0000
> +++ src/core/dbus/asio/executor.cpp	2014-06-05 14:12:25 +0000
> @@ -331,7 +331,8 @@
>      }
>  
>  public:
> -    explicit Executor(const Bus::Ptr& bus) : bus(bus), work(io_service)
> +
> +    Executor(const Bus::Ptr& bus, boost::asio::io_service& io, bool requires_free) : bus(bus), io_service(io), work(io_service), requires_free(requires_free)
>      {
>          if (!bus)
>              throw std::runtime_error("Precondition violated, cannot construct executor for null bus.");
> @@ -364,6 +365,8 @@
>      ~Executor() noexcept
>      {
>          stop();
> +        if (requires_free)

We really shouldn't do this :) Instead, we should resort to a static instance of io_service declared in make_executor(Bus::Ptr).

> +            delete &io_service;
>      }
>  
>      void run()
> @@ -378,13 +381,20 @@
>  
>  private:
>      Bus::Ptr bus;
> -    boost::asio::io_service io_service;
> +    boost::asio::io_service& io_service;
>      boost::asio::io_service::work work;
> +    bool requires_free;

See comment before, we shouldn't do this.

>  };
>  
>  ORG_FREEDESKTOP_DBUS_DLL_PUBLIC Executor::Ptr make_executor(const Bus::Ptr& bus)
>  {
> -    return std::make_shared<core::dbus::asio::Executor>(bus);
> +    auto io = new boost::asio::io_service();
> +    return std::make_shared<core::dbus::asio::Executor>(bus, *io, true);
> +}
> +
> +ORG_FREEDESKTOP_DBUS_DLL_PUBLIC Executor::Ptr make_executor(const Bus::Ptr& bus, boost::asio::io_service& io)
> +{
> +    return std::make_shared<core::dbus::asio::Executor>(bus, io, false);
>  }
>  
>  }
> 


-- 
https://code.launchpad.net/~mandel/dbus-cpp/expose_executor/+merge/222200
Your team Ubuntu Phablet Team is subscribed to branch lp:dbus-cpp.



More information about the Ubuntu-reviews mailing list