[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