[libcamera-devel] [PATCH] libcamera: base: shared_fd: Don't dup() an invalid fd

[Laurent Pinchart]

On Mon, Dec 06, 2021 at 03:26:30PM +0000, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2021-12-06 14: 07:01)
> > The SharedFD: :dup() implementation calls the C library dup() function on
> Date: Mon, 06 Dec 2021 15:26:27 +0000
> Message-ID: <163880438747.995700.268589314019276531 at Monstersaurus>
> User-Agent: alot/0.10
> 
> > the fd. When the SharedFD instance is invalid, this produces an invalid
> > fd, which is the correct behaviour, but logs an error message.
> 
> Isn't logging an error in that event a reasonable thing to do?

There could be valid use cases for dup() on an invalid SharedFD, I'm not
sure.

> > Fix it by returning an invalid UniqueFD directly when the SharedFD is
> > invalid. This also saves a system call, which is always nice to do.
> 
> Saving that seems worthwhile anyway as we know it won't succeed.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> > Fixes: fcf98514cb4e ("libcamera: base: file_descriptor: Return UniqueFD from dup()")
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/libcamera/base/shared_fd.cpp | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/src/libcamera/base/shared_fd.cpp b/src/libcamera/base/shared_fd.cpp
> > index bd2ab5aa25b7..c711cf579a04 100644
> > --- a/src/libcamera/base/shared_fd.cpp
> > +++ b/src/libcamera/base/shared_fd.cpp
> > @@ -253,6 +253,9 @@ SharedFD &SharedFD::operator=(SharedFD &&other)
> >   */
> >  UniqueFD SharedFD::dup() const
> >  {
> > +       if (!isValid())
> > +               return {};
> > +
> >         UniqueFD dupFd(::dup(get()));
> >         if (!dupFd.isValid()) {
> >                 int ret = -errno;
> > 
> > base-commit: f8d2f17a3db5ac490d7b505253a98d6795c2b630

-- 
Regards,

Laurent Pinchart