[libcamera-devel] [PATCH/RFC] meson: Remove -Wno-unused-parameter
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Nov 8 20:42:27 CET 2019
Hello everybody,
On Mon, Oct 28, 2019 at 12:35:44PM +0200, Laurent Pinchart wrote:
> On Mon, Oct 28, 2019 at 09:32:14AM +0000, Kieran Bingham wrote:
> > On 26/10/2019 22:38, Laurent Pinchart wrote:
> > > We build libcamera with -Wno-unused-parameter and this doesn't cause
> > > much issue internally. However, it prevents catching unused parameters
> > > in inline functions defined in public headers. This can lead to
> > > compilation warnings for applications compiled without
> > > -Wno-unused-parameter.
> > >
> > > To catch those issues, remove -Wno-unused-parameter and fix all the
> > > related warnings.
> >
> > What's your opinion on defining an unused(variable) (named otherwise if
> > required) macro so that we can declare unused variables in the code
> > base, and keep the function prototypes clean?
> >
> > i.e.
> >
> > #define unused(_x) (void)(_x)
> >
> > ControlValue ControlValue::load(ByteStreamBuffer &b, ControlType type,
> > unsigned int count)
> > {
> > + unused(count);
> > +
> > switch (type) {
> > case ControlTypeBool: {
> > bool value;
> > ...
> > }
> >
> > I think something like that would allow cleaner function declarations,
> > that do not need to be modified once someone actually uses the variable
> > in question. Simply remove the 'unused' declaration ?
>
> I'm afraid I don't like that :-) First of all, compilers may not always
> be fooled by that particular implementation, and we'll play a game of
> whack-a-mole as new compiler versions are released. Then, removing the
> variable name is specified by the C++ standard as the official way to
> note that a variable is unused. I don't think we should use a different,
> non-standard implementation.
>
> This patch stems from a warning generated when compiling a small test
> code against libcamera out of the libcamera source tree, without
> -Wno-unused-parameter. It turns out we had a single issue in public
> headers, so we will likely not introduce other such issues in the
> headers very often. We could thus decide not to care about it and catch
> the issues when they are reported. I however think there's value in
> dropping the compiler option though (otherwise I wouldn't have sent this
> patch in the first place :-)).
Ping ? I'd like to reach a consensus on this and decide if we should
drop the patch (and thus live with the small but real risk that
applications that use -Wunused-parameters would get a build warning) or
keep it, in this form or another.
> > > Fix an additional checkstyle.py error while at it.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > > meson.build | 1 -
> > > src/android/camera3_hal.cpp | 9 +++++----
> > > src/android/camera_device.cpp | 4 ++--
> > > src/android/camera_proxy.cpp | 4 ++--
> > > src/cam/main.cpp | 2 +-
> > > src/ipa/ipa_vimc.cpp | 10 +++++-----
> > > src/ipa/rkisp1/rkisp1.cpp | 2 +-
> > > src/libcamera/device_enumerator_udev.cpp | 2 +-
> > > src/libcamera/ipc_unixsocket.cpp | 2 +-
> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
> > > src/libcamera/pipeline/rkisp1/timeline.cpp | 2 +-
> > > src/libcamera/pipeline/uvcvideo.cpp | 2 +-
> > > src/libcamera/pipeline/vimc.cpp | 4 ++--
> > > src/libcamera/process.cpp | 2 +-
> > > src/libcamera/proxy/ipa_proxy_linux.cpp | 12 ++++++------
> > > src/libcamera/v4l2_videodevice.cpp | 2 +-
> > > src/qcam/main.cpp | 2 +-
> > > test/camera/capture.cpp | 2 +-
> > > test/libtest/test.h | 4 ++--
> > > test/log/log_process.cpp | 3 ++-
> > > test/message.cpp | 2 +-
> > > test/process/process_test.cpp | 3 ++-
> > > test/timer-thread.cpp | 2 +-
> > > test/timer.cpp | 2 +-
> > > 25 files changed, 43 insertions(+), 41 deletions(-)
> > >
> > > diff --git a/meson.build b/meson.build
> > > index 72ad7c8b493b..19a921a8ba6a 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -31,7 +31,6 @@ if cc.has_header_symbol('stdlib.h', 'secure_getenv', prefix : '#define _GNU_SOUR
> > > endif
> > >
> > > common_arguments = [
> > > - '-Wno-unused-parameter',
> > > '-include', 'config.h',
> > > ]
> > >
> > > diff --git a/src/android/camera3_hal.cpp b/src/android/camera3_hal.cpp
> > > index 8d2629ca356c..a7f470172583 100644
> > > --- a/src/android/camera3_hal.cpp
> > > +++ b/src/android/camera3_hal.cpp
> > > @@ -31,18 +31,19 @@ static int hal_get_camera_info(int id, struct camera_info *info)
> > > return cameraManager.getCameraInfo(id, info);
> > > }
> > >
> > > -static int hal_set_callbacks(const camera_module_callbacks_t *callbacks)
> > > +static int hal_set_callbacks(const camera_module_callbacks_t * /*callbacks*/)
> > > {
> > > return 0;
> > > }
> > >
> > > -static int hal_open_legacy(const struct hw_module_t *module, const char *id,
> > > - uint32_t halVersion, struct hw_device_t **device)
> > > +static int hal_open_legacy(const struct hw_module_t * /*module*/,
> > > + const char * /*id*/, uint32_t /*halVersion*/,
> > > + struct hw_device_t ** /*device*/)
> > > {
> > > return -ENOSYS;
> > > }
> > >
> > > -static int hal_set_torch_mode(const char *camera_id, bool enabled)
> > > +static int hal_set_torch_mode(const char * /*camera_id*/, bool /*enabled*/)
> > > {
> > > return -ENOSYS;
> > > }
> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > > index c7c9b3fd1724..b0ba1cf0a921 100644
> > > --- a/src/android/camera_device.cpp
> > > +++ b/src/android/camera_device.cpp
> > > @@ -49,7 +49,7 @@ CameraDevice::Camera3RequestDescriptor::~Camera3RequestDescriptor()
> > > * to the framework using the designated callbacks.
> > > */
> > >
> > > -CameraDevice::CameraDevice(unsigned int id, const std::shared_ptr<Camera> &camera)
> > > +CameraDevice::CameraDevice(unsigned int /*id*/, const std::shared_ptr<Camera> &camera)
> > > : running_(false), camera_(camera), staticMetadata_(nullptr)
> > > {
> > > camera_->requestCompleted.connect(this, &CameraDevice::requestComplete);
> > > @@ -875,7 +875,7 @@ void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream)
> > > /*
> > > * Produce a set of fixed result metadata.
> > > */
> > > -std::unique_ptr<CameraMetadata> CameraDevice::getResultMetadata(int frame_number,
> > > +std::unique_ptr<CameraMetadata> CameraDevice::getResultMetadata(int /*frame_number*/,
> > > int64_t timestamp)
> > > {
> > > /*
> > > diff --git a/src/android/camera_proxy.cpp b/src/android/camera_proxy.cpp
> > > index 4f5c0a024903..c9f98c784eec 100644
> > > --- a/src/android/camera_proxy.cpp
> > > +++ b/src/android/camera_proxy.cpp
> > > @@ -79,11 +79,11 @@ static int hal_dev_process_capture_request(const struct camera3_device *dev,
> > > return proxy->processCaptureRequest(request);
> > > }
> > >
> > > -static void hal_dev_dump(const struct camera3_device *dev, int fd)
> > > +static void hal_dev_dump(const struct camera3_device * /*dev*/, int /*fd*/)
> > > {
> > > }
> > >
> > > -static int hal_dev_flush(const struct camera3_device *dev)
> > > +static int hal_dev_flush(const struct camera3_device * /*dev*/)
> > > {
> > > return 0;
> > > }
> > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> > > index 9d99f5587cbb..ceb0efeff045 100644
> > > --- a/src/cam/main.cpp
> > > +++ b/src/cam/main.cpp
> > > @@ -326,7 +326,7 @@ int CamApp::run()
> > > return 0;
> > > }
> > >
> > > -void signalHandler(int signal)
> > > +void signalHandler(int /*signal*/)
> > > {
> > > std::cout << "Exiting" << std::endl;
> > > CamApp::instance()->quit();
> > > diff --git a/src/ipa/ipa_vimc.cpp b/src/ipa/ipa_vimc.cpp
> > > index 63d578b4e2aa..16c7e0732d40 100644
> > > --- a/src/ipa/ipa_vimc.cpp
> > > +++ b/src/ipa/ipa_vimc.cpp
> > > @@ -30,11 +30,11 @@ public:
> > > ~IPAVimc();
> > >
> > > int init() override;
> > > - void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> > > - const std::map<unsigned int, ControlInfoMap> &entityControls) override {}
> > > - void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
> > > - void unmapBuffers(const std::vector<unsigned int> &ids) override {}
> > > - void processEvent(const IPAOperationData &event) override {}
> > > + void configure(const std::map<unsigned int, IPAStream> & /*streamConfig*/,
> > > + const std::map<unsigned int, ControlInfoMap> & /*entityControls*/) override {}
> > > + void mapBuffers(const std::vector<IPABuffer> & /*buffers*/) override {}
> > > + void unmapBuffers(const std::vector<unsigned int> & /*ids*/) override {}
> > > + void processEvent(const IPAOperationData & /*event*/) override {}
> > >
> > > private:
> > > void initTrace();
> > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> > > index 9a13f5c7df17..84f270791340 100644
> > > --- a/src/ipa/rkisp1/rkisp1.cpp
> > > +++ b/src/ipa/rkisp1/rkisp1.cpp
> > > @@ -61,7 +61,7 @@ private:
> > > uint32_t maxGain_;
> > > };
> > >
> > > -void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,
> > > +void IPARkISP1::configure(const std::map<unsigned int, IPAStream> & /*streamConfig*/,
> > > const std::map<unsigned int, ControlInfoMap> &entityControls)
> > > {
> > > if (entityControls.empty())
> > > diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
> > > index b2c5fd221dcd..6e0fbbd8c70c 100644
> > > --- a/src/libcamera/device_enumerator_udev.cpp
> > > +++ b/src/libcamera/device_enumerator_udev.cpp
> > > @@ -309,7 +309,7 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)
> > > return 0;
> > > }
> > >
> > > -void DeviceEnumeratorUdev::udevNotify(EventNotifier *notifier)
> > > +void DeviceEnumeratorUdev::udevNotify(EventNotifier * /*notifier*/)
> > > {
> > > struct udev_device *dev = udev_monitor_receive_device(monitor_);
> > > std::string action(udev_device_get_action(dev));
> > > diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp
> > > index def08eef00f8..eb594268dd6c 100644
> > > --- a/src/libcamera/ipc_unixsocket.cpp
> > > +++ b/src/libcamera/ipc_unixsocket.cpp
> > > @@ -306,7 +306,7 @@ int IPCUnixSocket::recvData(void *buffer, size_t length,
> > > return 0;
> > > }
> > >
> > > -void IPCUnixSocket::dataNotifier(EventNotifier *notifier)
> > > +void IPCUnixSocket::dataNotifier(EventNotifier * /*notifier*/)
> > > {
> > > int ret;
> > >
> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > index 8d3ad568d16e..84356646b736 100644
> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > @@ -700,7 +700,7 @@ error:
> > > }
> > >
> > > int PipelineHandlerIPU3::freeBuffers(Camera *camera,
> > > - const std::set<Stream *> &streams)
> > > + const std::set<Stream *> & /*streams*/)
> > > {
> > > IPU3CameraData *data = cameraData(camera);
> > >
> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > index 7a28b03b8d38..aed060bada70 100644
> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > > @@ -703,7 +703,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,
> > > }
> > >
> > > int PipelineHandlerRkISP1::freeBuffers(Camera *camera,
> > > - const std::set<Stream *> &streams)
> > > + const std::set<Stream *> & /*streams*/)
> > > {
> > > RkISP1CameraData *data = cameraData(camera);
> > >
> > > diff --git a/src/libcamera/pipeline/rkisp1/timeline.cpp b/src/libcamera/pipeline/rkisp1/timeline.cpp
> > > index f6c6434d7b53..59e6de78c79d 100644
> > > --- a/src/libcamera/pipeline/rkisp1/timeline.cpp
> > > +++ b/src/libcamera/pipeline/rkisp1/timeline.cpp
> > > @@ -204,7 +204,7 @@ void Timeline::updateDeadline()
> > > timer_.start(deadline);
> > > }
> > >
> > > -void Timeline::timeout(Timer *timer)
> > > +void Timeline::timeout(Timer * /*timer*/)
> > > {
> > > utils::time_point now = std::chrono::steady_clock::now();
> > >
> > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> > > index fae0ffc4de30..94464c7c7f0c 100644
> > > --- a/src/libcamera/pipeline/uvcvideo.cpp
> > > +++ b/src/libcamera/pipeline/uvcvideo.cpp
> > > @@ -208,7 +208,7 @@ int PipelineHandlerUVC::allocateBuffers(Camera *camera,
> > > }
> > >
> > > int PipelineHandlerUVC::freeBuffers(Camera *camera,
> > > - const std::set<Stream *> &streams)
> > > + const std::set<Stream *> & /*streams*/)
> > > {
> > > UVCCameraData *data = cameraData(camera);
> > > return data->video_->releaseBuffers();
> > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> > > index c16ae4cb76b5..7e325469f178 100644
> > > --- a/src/libcamera/pipeline/vimc.cpp
> > > +++ b/src/libcamera/pipeline/vimc.cpp
> > > @@ -166,7 +166,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)
> > > {
> > > }
> > >
> > > -CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
> > > +CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera * /*camera*/,
> > > const StreamRoles &roles)
> > > {
> > > CameraConfiguration *config = new VimcCameraConfiguration();
> > > @@ -274,7 +274,7 @@ int PipelineHandlerVimc::allocateBuffers(Camera *camera,
> > > }
> > >
> > > int PipelineHandlerVimc::freeBuffers(Camera *camera,
> > > - const std::set<Stream *> &streams)
> > > + const std::set<Stream *> & /*streams*/)
> > > {
> > > VimcCameraData *data = cameraData(camera);
> > > return data->video_->releaseBuffers();
> > > diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
> > > index 3b4d0f10da67..44ebfec178fe 100644
> > > --- a/src/libcamera/process.cpp
> > > +++ b/src/libcamera/process.cpp
> > > @@ -89,7 +89,7 @@ void sigact(int signal, siginfo_t *info, void *ucontext)
> > >
> > > } /* namespace */
> > >
> > > -void ProcessManager::sighandler(EventNotifier *notifier)
> > > +void ProcessManager::sighandler(EventNotifier * /*notifier*/)
> > > {
> > > char data;
> > > ssize_t ret = read(pipe_[0], &data, sizeof(data));
> > > diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
> > > index 4e6fa6899e07..d4ccf5112cd7 100644
> > > --- a/src/libcamera/proxy/ipa_proxy_linux.cpp
> > > +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
> > > @@ -27,11 +27,11 @@ public:
> > > ~IPAProxyLinux();
> > >
> > > int init() override { return 0; }
> > > - void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> > > - const std::map<unsigned int, ControlInfoMap> &entityControls) override {}
> > > - void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
> > > - void unmapBuffers(const std::vector<unsigned int> &ids) override {}
> > > - void processEvent(const IPAOperationData &event) override {}
> > > + void configure(const std::map<unsigned int, IPAStream> & /*streamConfig*/,
> > > + const std::map<unsigned int, ControlInfoMap> & /*entityControls*/) override {}
> > > + void mapBuffers(const std::vector<IPABuffer> & /*buffers*/) override {}
> > > + void unmapBuffers(const std::vector<unsigned int> & /*ids*/) override {}
> > > + void processEvent(const IPAOperationData & /*event*/) override {}
> > >
> > > private:
> > > void readyRead(IPCUnixSocket *ipc);
> > > @@ -86,7 +86,7 @@ IPAProxyLinux::~IPAProxyLinux()
> > > delete socket_;
> > > }
> > >
> > > -void IPAProxyLinux::readyRead(IPCUnixSocket *ipc)
> > > +void IPAProxyLinux::readyRead(IPCUnixSocket * /*ipc*/)
> > > {
> > > }
> > >
> > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > > index 208ab54199b1..87d810d7cfa4 100644
> > > --- a/src/libcamera/v4l2_videodevice.cpp
> > > +++ b/src/libcamera/v4l2_videodevice.cpp
> > > @@ -1141,7 +1141,7 @@ Buffer *V4L2VideoDevice::dequeueBuffer()
> > > * For Capture video devices the Buffer will contain valid data.
> > > * For Output video devices the Buffer can be considered empty.
> > > */
> > > -void V4L2VideoDevice::bufferAvailable(EventNotifier *notifier)
> > > +void V4L2VideoDevice::bufferAvailable(EventNotifier * /*notifier*/)
> > > {
> > > Buffer *buffer = dequeueBuffer();
> > > if (!buffer)
> > > diff --git a/src/qcam/main.cpp b/src/qcam/main.cpp
> > > index a7ff5c52663b..d8cfc1c3d76b 100644
> > > --- a/src/qcam/main.cpp
> > > +++ b/src/qcam/main.cpp
> > > @@ -17,7 +17,7 @@
> > > #include "../cam/options.h"
> > > #include "qt_event_dispatcher.h"
> > >
> > > -void signalHandler(int signal)
> > > +void signalHandler(int /*signal*/)
> > > {
> > > std::cout << "Exiting" << std::endl;
> > > qApp->quit();
> > > diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> > > index 791ccad15f70..a5fd0a641705 100644
> > > --- a/test/camera/capture.cpp
> > > +++ b/test/camera/capture.cpp
> > > @@ -19,7 +19,7 @@ protected:
> > > unsigned int completeBuffersCount_;
> > > unsigned int completeRequestsCount_;
> > >
> > > - void bufferComplete(Request *request, Buffer *buffer)
> > > + void bufferComplete(Request * /*request*/, Buffer *buffer)
> > > {
> > > if (buffer->status() != Buffer::BufferSuccess)
> > > return;
> > > diff --git a/test/libtest/test.h b/test/libtest/test.h
> > > index ec08bf97c03d..193d7aa99f38 100644
> > > --- a/test/libtest/test.h
> > > +++ b/test/libtest/test.h
> > > @@ -26,11 +26,11 @@ public:
> > > protected:
> > > virtual int init() { return 0; }
> > > virtual int run() = 0;
> > > - virtual void cleanup() { }
> > > + virtual void cleanup() {}
> > > };
> > >
> > > #define TEST_REGISTER(klass) \
> > > -int main(int argc, char *argv[]) \
> > > +int main(int /*argc*/, char * /*argv*/[]) \
> > > { \
> > > return klass().execute(); \
> > > }
> > > diff --git a/test/log/log_process.cpp b/test/log/log_process.cpp
> > > index 2df4aa43713c..fa5639dde7cb 100644
> > > --- a/test/log/log_process.cpp
> > > +++ b/test/log/log_process.cpp
> > > @@ -123,7 +123,8 @@ protected:
> > > }
> > >
> > > private:
> > > - void procFinished(Process *proc, enum Process::ExitStatus exitStatus, int exitCode)
> > > + void procFinished(Process * /*proc*/,
> > > + enum Process::ExitStatus exitStatus, int exitCode)
> > > {
> > > exitStatus_ = exitStatus;
> > > exitCode_ = exitCode;
> > > diff --git a/test/message.cpp b/test/message.cpp
> > > index 3775c30a20b3..3e2659c836e3 100644
> > > --- a/test/message.cpp
> > > +++ b/test/message.cpp
> > > @@ -35,7 +35,7 @@ public:
> > > void reset() { status_ = NoMessage; }
> > >
> > > protected:
> > > - void message(Message *msg)
> > > + void message(Message * /*msg*/)
> > > {
> > > if (thread() != Thread::current())
> > > status_ = InvalidThread;
> > > diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp
> > > index 7e7b3c2c8bf3..2ba3efd08457 100644
> > > --- a/test/process/process_test.cpp
> > > +++ b/test/process/process_test.cpp
> > > @@ -75,7 +75,8 @@ protected:
> > > }
> > >
> > > private:
> > > - void procFinished(Process *proc, enum Process::ExitStatus exitStatus, int exitCode)
> > > + void procFinished(Process * /*proc*/,
> > > + enum Process::ExitStatus exitStatus, int exitCode)
> > > {
> > > exitStatus_ = exitStatus;
> > > exitCode_ = exitCode;
> > > diff --git a/test/timer-thread.cpp b/test/timer-thread.cpp
> > > index 32853b4e80ef..55b7abcbbeab 100644
> > > --- a/test/timer-thread.cpp
> > > +++ b/test/timer-thread.cpp
> > > @@ -39,7 +39,7 @@ public:
> > > }
> > >
> > > private:
> > > - void timeoutHandler(Timer *timer)
> > > + void timeoutHandler(Timer * /*timer*/)
> > > {
> > > timeout_ = true;
> > > }
> > > diff --git a/test/timer.cpp b/test/timer.cpp
> > > index 2bdb006edccb..03df03aa8d69 100644
> > > --- a/test/timer.cpp
> > > +++ b/test/timer.cpp
> > > @@ -56,7 +56,7 @@ public:
> > > }
> > >
> > > private:
> > > - void timeoutHandler(Timer *timer)
> > > + void timeoutHandler(Timer * /*timer*/)
> > > {
> > > expiration_ = std::chrono::steady_clock::now();
> > > count_++;
> > >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list