| Message ID | 20240201050810.3501276-1-raj.khem@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hello Khem, Thank you for the patch. On Wed, Jan 31, 2024 at 09:08:09PM -0800, Khem Raj wrote: > unlock uses lockf which is marked with __attribute__ s/unlock/MediaDevice::unlock()/ > ((warn_unused_result)) and compilers warn about it and some treat > -Wunused-result as error with -Werror turned on, It would be good to > check if lockf failed or succeeded, however, that piece is not changed > with this, this fixes build with clang++ 18 I wouldn't mention clang in the commit message, it's not a compiler-specific issue, all compilers will warn about the unused result. Where does that unused result attribute come from though ? I don't see it in glibc : https://sourceware.org/git/?p=glibc.git;a=blob;f=io/fcntl.h;h=9cee0b5900cb4f7c3bd94344e17824c843815e3d;hb=HEAD#l274 > > ../git/src/libcamera/media_device.cpp:167:2: error: ignoring return value of function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result] > 167 | lockf(fd_.get(), F_ULOCK, 0); > | ^~~~~ ~~~~~~~~~~~~~~~~~~~~~ > 1 error generated. > > Signed-off-by: Khem Raj <raj.khem@gmail.com> > --- > include/libcamera/internal/media_device.h | 2 +- > src/libcamera/media_device.cpp | 6 +++--- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/include/libcamera/internal/media_device.h b/include/libcamera/internal/media_device.h > index eb8cfde4..b09dfd16 100644 > --- a/include/libcamera/internal/media_device.h > +++ b/include/libcamera/internal/media_device.h > @@ -33,7 +33,7 @@ public: > bool busy() const { return acquired_; } > > bool lock(); > - void unlock(); > + bool unlock(); > > int populate(); > bool isValid() const { return valid_; } > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp > index 2949816b..eaa2fdb0 100644 > --- a/src/libcamera/media_device.cpp > +++ b/src/libcamera/media_device.cpp > @@ -159,12 +159,12 @@ bool MediaDevice::lock() > * > * \sa lock() > */ > -void MediaDevice::unlock() > +bool MediaDevice::unlock() > { > if (!fd_.isValid()) > - return; > + return false; > > - lockf(fd_.get(), F_ULOCK, 0); > + return lockf(fd_.get(), F_ULOCK, 0) == 0; If you don't check the return value in the callers this is just a hack to silence the warning, in which case you may as well silence it here without changing the return value. > } > > /**
diff --git a/include/libcamera/internal/media_device.h b/include/libcamera/internal/media_device.h index eb8cfde4..b09dfd16 100644 --- a/include/libcamera/internal/media_device.h +++ b/include/libcamera/internal/media_device.h @@ -33,7 +33,7 @@ public: bool busy() const { return acquired_; } bool lock(); - void unlock(); + bool unlock(); int populate(); bool isValid() const { return valid_; } diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index 2949816b..eaa2fdb0 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -159,12 +159,12 @@ bool MediaDevice::lock() * * \sa lock() */ -void MediaDevice::unlock() +bool MediaDevice::unlock() { if (!fd_.isValid()) - return; + return false; - lockf(fd_.get(), F_ULOCK, 0); + return lockf(fd_.get(), F_ULOCK, 0) == 0; } /**
unlock uses lockf which is marked with __attribute__ ((warn_unused_result)) and compilers warn about it and some treat -Wunused-result as error with -Werror turned on, It would be good to check if lockf failed or succeeded, however, that piece is not changed with this, this fixes build with clang++ 18 ../git/src/libcamera/media_device.cpp:167:2: error: ignoring return value of function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result] 167 | lockf(fd_.get(), F_ULOCK, 0); | ^~~~~ ~~~~~~~~~~~~~~~~~~~~~ 1 error generated. Signed-off-by: Khem Raj <raj.khem@gmail.com> --- include/libcamera/internal/media_device.h | 2 +- src/libcamera/media_device.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)