[PATCH 1/1] DmaBufAllocator: Make DmaSyncer non-copyable

[Cheng-Hao Yang]

Hi Kieran,

On Wed, Dec 11, 2024 at 1:25 AM Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Quoting Harvey Yang (2024-11-29 18:53:28)
> > This fixes commit 39482d59fe7160740ea5dd61f3ed965a88d848ce.
>
> Have you seen how we style fixes tags to match the linux kernel?
>
> Taken from: https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
>
>
> """
>
> If your patch fixes a bug in a specific commit, e.g. you found an issue
> using git bisect, please use the ‘Fixes:’ tag with the first 12
> characters of the SHA-1 ID, and the one line summary. Do not split the
> tag across multiple lines, tags are exempt from the “wrap at 75 columns”
> rule in order to simplify parsing scripts. For example:
>
> Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed")
>
> The following git config settings can be used to add a pretty format for
> outputting the above style in the git log or git show commands:
>
> [core]
>         abbrev = 12
> [pretty]
>         fixes = Fixes: %h (\"%s\")
>
> An example call:
>
> $ git log -1 --pretty=fixes 54a4f0239f2e
> Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed")
>

Thanks for the suggestion!
Added in my git config, and sent a new version.

BR,
Harvey

> """
>
>
>
> >
> > As DmaSyncer does sync start/end in the c'tor/d'tor, copying a DmaSyncer
> > instance would trigger sync end earlier than expected. This patch makes
> > it non-copyable to avoid the issue.
> >
> > Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
>
> With the fixes tag corrected
>
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > ---
> >  include/libcamera/internal/dma_buf_allocator.h |  5 +++++
> >  src/libcamera/dma_buf_allocator.cpp            | 10 ++++++++++
> >  2 files changed, 15 insertions(+)
> >
> > diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h
> > index fc5de2c13..0cb209630 100644
> > --- a/include/libcamera/internal/dma_buf_allocator.h
> > +++ b/include/libcamera/internal/dma_buf_allocator.h
> > @@ -60,9 +60,14 @@ public:
> >
> >         explicit DmaSyncer(SharedFD fd, SyncType type = SyncType::ReadWrite);
> >
> > +       DmaSyncer(DmaSyncer &&) = default;
> > +       DmaSyncer &operator=(DmaSyncer &&) = default;
> > +
> >         ~DmaSyncer();
> >
> >  private:
> > +       LIBCAMERA_DISABLE_COPY(DmaSyncer)
> > +
> >         void sync(uint64_t step);
> >
> >         SharedFD fd_;
> > diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp
> > index 3cc52f968..5f517828d 100644
> > --- a/src/libcamera/dma_buf_allocator.cpp
> > +++ b/src/libcamera/dma_buf_allocator.cpp
> > @@ -311,6 +311,16 @@ DmaSyncer::DmaSyncer(SharedFD fd, SyncType type)
> >         sync(DMA_BUF_SYNC_START);
> >  }
> >
> > +/**
> > + * \fn DmaSyncer::DmaSyncer(DmaSyncer &&) = default;
> > + * \brief Enable move on class DmaSyncer
> > + */
> > +
> > +/**
> > + * \fn DmaSyncer::operator=(DmaSyncer &&) = default;
> > + * \brief Enable move on class DmaSyncer
> > + */
> > +
> >  DmaSyncer::~DmaSyncer()
> >  {
> >         sync(DMA_BUF_SYNC_END);
> > --
> > 2.47.0.338.g60cca15819-goog
> >