[PATCH v6 05/18] libcamera: shared_mem_object: reorganize the code and document the SharedMemObject class

[Kieran Bingham]

Quoting Milan Zamazal (2024-04-02 20:12:51)
> Hi Andrei,
> 
> thank you for clarification.
> 
> Andrei Konovalov <andrey.konovalov.ynk at gmail.com> writes:
> 
> > Hi Milan,
> >
> > On 28.03.2024 12:43, Milan Zamazal wrote:
> >> Andrei, Dennis,
> >> could you please help me with the points below?
> >> Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:
> >> Thank you, Laurent, for the review and all the suggestions regarding docstring
> >> improvements (very nice and educative!) etc.
> >> 
> >>>> Split the parts which doesn't otherwise depend on the type T or
> >>>> arguments Args out of the SharedMemObject class into a new
> >>>> SharedMem class.
> >>>
> >>> The commit message should explain why.
> >> Do you have a suggestion what to write there?
> >
> > This split was suggested by Laurent in his review of the v2 patch:
> > https://lists.libcamera.org/pipermail/libcamera-devel/2024-January/040344.html
> >
> > My understanding was that we shouldn't create an extra copy of the same code
> > per each type T.
> 
> In such a case, I don't have any idea what smart to add there, it looks saying
> basically the thing.
> 
> >>>> +
> >>>> +  /* Make SharedMem non-copyable for now. */
> >>>
> >>> Record the reason in the commit message, and drop this comment.
> >> The same here.
> >
> > This comment comes from the original code by RaspberryPi.
> 
> I couldn't find the reason, so I left this unchanged.

What would it mean to copy a SharedMem object? Would the copy simply be
a lightweight copy of the SharedMem class with an increased reference?
(like copying a shared pointer) Or does it make a clean copy of the
underlying SharedMem?

I suspect answering that question answers the why it's non-copyable. Or
maybe because that question isn't answered is why it's set as
non-copyable....

> [...]
> 
> >>>> +  if (ftruncate(fd_.get(), size_) < 0)
> >>>> +          return;
> >>>
> >>> Should we set the GROW and SHRINK seals (in a separate patch) ?
> >
> > Yes, this can be done.
> > Setting F_SEAL_SHRINK and F_SEAL_GROW after the ftruncate() call above could catch
> > some potential errors related to improper access to the shared memory allocated by
> > the SharedMemObject.
> 
> Added to TODO.
>