[PATCH v7 05/18] libcamera: shared_mem_object: Rename SIZE constant to `size'

[Laurent Pinchart]

Hello,

On Mon, Apr 08, 2024 at 05:40:33PM +0200, Milan Zamazal wrote:
> Kieran Bingham <kieran.bingham at ideasonboard.com> writes:
> 
> > Quoting Milan Zamazal (2024-04-04 09:46:42)
> >
> > Potential expansion to add (while committing??)
> >
> > The SharedMemObject has been imported directly into the libcamera
> > internal components. Adapt the SIZE constant of the class to match the
> > libcamera coding style.
> >
> >
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >
> >
> > That said, don't we prefix constants with a k? kSize ? Or maybe it's not
> > quite globally constant as it's only constant to the class
> > instantiation?
> >
> > And as it's a class variable should it be postfixed with an underscore?
> >
> > 	static constexpr kSize_ = sizeof(T); ?

It should be prefixed with a k, yes. As for the trailing underscore, I
don't think we use it for constants.

With kSize,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> Right, I'll fix it if we decide for v8, otherwise I can fix it in a followup
> patch.

There are minor review comments on other patches, so I think a final v8
will be needed. I'm worried I would be messing up something if I applied
all the small changes myself.

> >> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> >> ---
> >>  include/libcamera/internal/shared_mem_object.h | 8 ++++----
> >>  1 file changed, 4 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/include/libcamera/internal/shared_mem_object.h b/include/libcamera/internal/shared_mem_object.h
> >> index 98636b44..8f2e7120 100644
> >> --- a/include/libcamera/internal/shared_mem_object.h
> >> +++ b/include/libcamera/internal/shared_mem_object.h
> >> @@ -23,7 +23,7 @@ template<class T>
> >>  class SharedMemObject
> >>  {
> >>  public:
> >> -       static constexpr std::size_t SIZE = sizeof(T);
> >> +       static constexpr std::size_t size = sizeof(T);
> >>  
> >>         SharedMemObject()
> >>                 : obj_(nullptr)
> >> @@ -45,11 +45,11 @@ public:
> >>                 if (!fd_.isValid())
> >>                         return;
> >>  
> >> -               ret = ftruncate(fd_.get(), SIZE);
> >> +               ret = ftruncate(fd_.get(), size);
> >>                 if (ret < 0)
> >>                         return;
> >>  
> >> -               mem = mmap(nullptr, SIZE, PROT_READ | PROT_WRITE, MAP_SHARED,
> >> +               mem = mmap(nullptr, size, PROT_READ | PROT_WRITE, MAP_SHARED,
> >>                            fd_.get(), 0);
> >>                 if (mem == MAP_FAILED)
> >>                         return;
> >> @@ -69,7 +69,7 @@ public:
> >>         {
> >>                 if (obj_) {
> >>                         obj_->~T();
> >> -                       munmap(obj_, SIZE);
> >> +                       munmap(obj_, size);
> >>                 }
> >>         }
> >>  
> 

-- 
Regards,

Laurent Pinchart