[PATCH v6 05/18] libcamera: shared_mem_object: reorganize the code and document the SharedMemObject class
Milan Zamazal
mzamazal at redhat.com
Tue Apr 2 21:12:51 CEST 2024
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.
[...]
>>>> + 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.
More information about the libcamera-devel
mailing list