Mir

Code review comment for lp:~albaguirre/mir/fix-1421255

Revision history for this message
Alberto Aguirre (albaguirre) wrote :

> 211 + [this] { guard_lock =
> std::move(std::unique_lock<decltype(guard)>{guard}); },
> 212 + []{});
> It seems strange that if I were to call:
> lock();
> unlock();
> lock()
> The second lock call would block, because we haven't called the callback,
> which is what will end up releasing the lock.

OK, I suppose I can change BufferQueue private methods to take a unique_lock reference instead
of value, so I don't have to move the instance and consequently I can use it in the "unlock" lambda.

> 594 + CallerAutoLock caller_lock{ctx.lock, ctx.unlock};
> Could mir/raii.h help reduce diff? (and don't need CallerAutoLock)

I tried, but couldn't figure out why paired_call did not like std::function. I'll try harder I suppose :)

> 99 + virtual std::unique_ptr<FrameDroppingPolicy>
> create_policy(std::function<void()> const& drop_frame) const = 0;
> 119 + virtual std::unique_ptr<FrameDroppingPolicy> create_policy(
> 120 + std::function<void()> const& drop_frame,
> 121 + std::function<void()> const& lock,
> 122 + std::function<void()> const& unlock) const = 0;
>
> I guess I'd rather just keep one function (the latter), and 3 parameters in
> create_policy seem like they're becoming an interface that meets the
> requirements of BasicLockable.

OK. Maybe a LockableCallable?

« Back to merge proposal