[libcamera-devel] [PATCH 2/5] libcamera: media_device: Add function to get a MediaLink

[Laurent Pinchart]

Hi Jacopo,

On Tuesday, 8 January 2019 15:49:33 EET Jacopo Mondi wrote:
> On Mon, Jan 07, 2019 at 11:39:43PM +0200, Laurent Pinchart wrote:
> > Hello,
> 
> [snip]
> 
> >>>> + */
> >>>> +MediaLink *MediaDevice::link(const std::string sourceName, unsigned
> >>>> int sourceIdx,
> >>>> +			     const std::string sinkName, unsigned int sinkIdx)
> >>>> const
> > 
> > Isn't it dangerous to create a function that allows modifying a link from
> > a const MediaDevice ? Should you provide both
> > 
> > 	MediaLink *MediaDevice::link(...)
> > 	const MediaLink *MediaDevice::link(...) const
> > 
> > ? If you don't want to provide both I think you could go for the former
> > only, as I don't think we'll have to deal with const MediaDevice
> > pointers.
> 
> The function is declared as:
> MediaLink *MediaDevice::link(...) const
> and not
> const MediaLink *MediaDevice::link(...) const
> 
> and indeed the function itself does not change the MediaDevice
> instance state. Would you drop the const at end of the declaration?

I think I would do so, as it's a bit misleading otherwise. True, the function 
doesn't change the MediaDevice instance itself, but it allows non-const access 
to a link, which is part of the graph. If we give a const MediaDevice * 
pointer to someone, I wouldn't expect the API to allow changing links.

[snip]

-- 
Regards,

Laurent Pinchart