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

[Milan Zamazal]

Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:

> 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.

Hi Laurent,

OK, I'll prepare v8.

Regards,
Milan

>> >> 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);
>> >>                 }
>> >>         }
>> >>  
>>