[libcamera-devel] [PATCH 02/10] ipa: add missing thread-safety annotations

[Nicholas Roth]

> How come this gets inserted here by git when the commit author and the
sender have the same identity ?

I'm really not sure. Maybe because I specify "--from" in git-send-email? Is
this not a standard thing to do?
git send-email --compose --from=nicholas at rothemail.net --reply-to=
libcamera-devel at lists.libcamera.org --to=libcamera-devel at lists.libcamera.org
--subject="libcamera Android Enhancements" --thread --no-chain-reply-to
--annotate -v2 main

> nit: double empty line
If you're referring to the commit message, the line below the error is for
the carat that Clang uses to indicate the position of the error. Happy to
not skip a line before "Signed-off-by" if it would help.

> this seems correct to me, even more so if it silences a compilation error.
Do I need an LGTM from anyone else? What would the next step be here?

On Thu, Oct 27, 2022 at 11:08 AM Jacopo Mondi <jacopo at jmondi.org> wrote:

> Hi Nicholas
>
> On Thu, Oct 27, 2022 at 12:55:07AM -0500, Nicholas Roth via
> libcamera-devel wrote:
> > From: Nicholas Roth <nicholas at rothemail.net>
>
> How come this gets inserted here by git when the commit author and the
> sender have the same identity ?
>
> >
> > The raspberrypi IPA is missing thread-safety annotations, which breaks
> > the build.
> >
> > Add required thread-safety annotations.
> >
> > ../src/ipa/raspberrypi/controller/metadata.h:108:31: error: mutex
> >   'mutex_' is still held at the end of function
> [-Werror,-Wthread-safety-analysis]
> >         void lock() { mutex_.lock(); }
> >                                      ^
> > ../src/ipa/raspberrypi/controller/metadata.h:108:23: note: mutex
> >   acquired here
> >         void lock() { mutex_.lock(); }
> >                              ^
> > ../src/ipa/raspberrypi/controller/metadata.h:109:25: error: releasing
> >   mutex 'mutex_' that was not held [-Werror,-Wthread-safety-analysis]
> >         void unlock() { mutex_.unlock(); }
> >                                ^
>
> nit: double empty line
>
> >
> > Signed-off-by: Nicholas Roth <nicholas at rothemail.net>
>
> I've not followed much the thread annotation thing, but this seems
> correct to me, even more so if it silences a compilation error.
>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> > ---
> >  src/ipa/raspberrypi/controller/metadata.h | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/controller/metadata.h
> b/src/ipa/raspberrypi/controller/metadata.h
> > index 0f7ebfaf..870b6e26 100644
> > --- a/src/ipa/raspberrypi/controller/metadata.h
> > +++ b/src/ipa/raspberrypi/controller/metadata.h
> > @@ -13,9 +13,11 @@
> >  #include <mutex>
> >  #include <string>
> >
> > +#include <libcamera/base/thread_annotations.h>
> > +
> >  namespace RPiController {
> >
> > -class Metadata
> > +class LIBCAMERA_TSA_CAPABILITY("mutex") Metadata
> >  {
> >  public:
> >       Metadata() = default;
> > @@ -103,8 +105,8 @@ public:
> >        * locks with the standard lock classes.
> >        * e.g. std::lock_guard<RPiController::Metadata> lock(metadata)
> >        */
> > -     void lock() { mutex_.lock(); }
> > -     void unlock() { mutex_.unlock(); }
> > +     void lock() LIBCAMERA_TSA_ACQUIRE() { mutex_.lock(); }
> > +     void unlock() LIBCAMERA_TSA_RELEASE() { mutex_.unlock(); }
> >
> >  private:
> >       mutable std::mutex mutex_;
> > --
> > 2.34.1
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20221027/0e5f941c/attachment.htm>