[libcamera-devel,PATCH/RFC] meson: Remove -Wno-unused-parameter

2229 diff mbox series

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.

Fix an additional checkstyle.py error while at it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@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(-)

Hi Laurent,

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 ?

--
Kieran


> Fix an additional checkstyle.py error while at it.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@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_++;
>

Hi Kieran,

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 :-)).

> > Fix an additional checkstyle.py error while at it.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@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_++;
> >

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@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_++;
> > >

Hi Laurent,

On Fri, Nov 08, 2019 at 09:42:27PM +0200, Laurent Pinchart wrote:
> 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.
>

I think the priority here is to make sure applications do not get
warnings when building with -Wunused-parameters, so I'm finw with
either removing the parameter's name completely from the function
definition or commenting it out, which I like less but mostly because
of how it looks, so I'm fine with both, as long as we make sure
applications do not get warnings...

Thanks
  j

> > > > Fix an additional checkstyle.py error while at it.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@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
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Laurent,

On Sat, Nov 09, 2019 at 11:31:35AM +0100, Jacopo Mondi wrote:
> Hi Laurent,
>

Sorry, I forgot to add my tag
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> On Fri, Nov 08, 2019 at 09:42:27PM +0200, Laurent Pinchart wrote:
> > 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.
> >
>
> I think the priority here is to make sure applications do not get
> warnings when building with -Wunused-parameters, so I'm finw with
> either removing the parameter's name completely from the function
> definition or commenting it out, which I like less but mostly because
> of how it looks, so I'm fine with both, as long as we make sure
> applications do not get warnings...
>
> Thanks
>   j
>
> > > > > Fix an additional checkstyle.py error while at it.
> > > > >
> > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@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
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel



> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi All,

On 18/11/2019 21:53, Jacopo Mondi wrote:
> Hi Laurent,
> 
> On Sat, Nov 09, 2019 at 11:31:35AM +0100, Jacopo Mondi wrote:
>> Hi Laurent,
>>
> 
> Sorry, I forgot to add my tag
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> Thanks
>   j
> 
>> On Fri, Nov 08, 2019 at 09:42:27PM +0200, Laurent Pinchart wrote:
>>> 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.

For clarification - I fully agree that removing -Wno-unused-parameter is
a good thing. I'm very much a -Wall -Wextra -Werror kind of guy. Fix all
the warnings, before someone else has to, and don't cause warnings for
other people.


In terms of style, I am not fond of changing the function prototype, so
here's a last ditch effort of suggesting an alternate Unused()
definition type based on templates which I think should always work?
(Because the argument is used, but optimised out)


From: https://stackoverflow.com/a/39746834/3660427

template <typename T>
void Unused(T&& /*No name*/) { /*Empty*/ }

void func(int i)
{
   Unused(i); // Silent warning for unused variable
}


And that's purely because I believe it's cleaner to remove the Unused()
line when you want to then /use/ that variable rather than modify the
function prototype.

But - I'll not nack this patch based on that.

It's a shame that adding the name in via comments adds too many *'s though:

> -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*/)

I fear the /*device*/ reduces readability (IMO), but if you believe this
is the best way to solve the topic, then:

Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>




>>>>
>>>> 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.
>>>
>>
>> I think the priority here is to make sure applications do not get
>> warnings when building with -Wunused-parameters, so I'm finw with
>> either removing the parameter's name completely from the function
>> definition or commenting it out, which I like less but mostly because
>> of how it looks, so I'm fine with both, as long as we make sure
>> applications do not get warnings...
>>
>> Thanks
>>   j
>>
>>>>>> Fix an additional checkstyle.py error while at it.
>>>>>>
>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@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
>>> _______________________________________________
>>> libcamera-devel mailing list
>>> libcamera-devel@lists.libcamera.org
>>> https://lists.libcamera.org/listinfo/libcamera-devel
> 
> 
> 
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel@lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
> 
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>

Hi Kieran,

On Tue, Nov 19, 2019 at 01:35:41PM +0000, Kieran Bingham wrote:
> On 18/11/2019 21:53, Jacopo Mondi wrote:
> > On Sat, Nov 09, 2019 at 11:31:35AM +0100, Jacopo Mondi wrote:
> >> Hi Laurent,
> > 
> > Sorry, I forgot to add my tag
> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> > 
> >> On Fri, Nov 08, 2019 at 09:42:27PM +0200, Laurent Pinchart wrote:
> >>> 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.
> 
> For clarification - I fully agree that removing -Wno-unused-parameter is
> a good thing. I'm very much a -Wall -Wextra -Werror kind of guy. Fix all
> the warnings, before someone else has to, and don't cause warnings for
> other people.
> 
> In terms of style, I am not fond of changing the function prototype, so
> here's a last ditch effort of suggesting an alternate Unused()
> definition type based on templates which I think should always work?
> (Because the argument is used, but optimised out)
> 
> 
> From: https://stackoverflow.com/a/39746834/3660427
> 
> template <typename T>
> void Unused(T&& /*No name*/) { /*Empty*/ }
> 
> void func(int i)
> {
>    Unused(i); // Silent warning for unused variable
> }
> 
> 
> And that's purely because I believe it's cleaner to remove the Unused()
> line when you want to then /use/ that variable rather than modify the
> function prototype.
> 
> But - I'll not nack this patch based on that.
> 
> It's a shame that adding the name in via comments adds too many *'s though:
> 
> > -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*/)
> 
> I fear the /*device*/ reduces readability (IMO), but if you believe this
> is the best way to solve the topic, then:
> 
> Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

The part that bothers me with the Unused() function above is that C++
provides a standard way to state a parameter is unused, and using a
different method seems a bit of a hack to me. There's also a small risk
that new compiler versions could still produce a warning if they get too
clever about our hacks :-S

If we want not to comment out parameter names we could use
__attribute__((__unused__)), but I think that would be even less
readable.

> >>>> 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.
> >>
> >> I think the priority here is to make sure applications do not get
> >> warnings when building with -Wunused-parameters, so I'm finw with
> >> either removing the parameter's name completely from the function
> >> definition or commenting it out, which I like less but mostly because
> >> of how it looks, so I'm fine with both, as long as we make sure
> >> applications do not get warnings...
> >>
> >>>>>> Fix an additional checkstyle.py error while at it.
> >>>>>>
> >>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@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_++;

Hi Laurent,

On 19/11/2019 22:25, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Tue, Nov 19, 2019 at 01:35:41PM +0000, Kieran Bingham wrote:
>> On 18/11/2019 21:53, Jacopo Mondi wrote:
>>> On Sat, Nov 09, 2019 at 11:31:35AM +0100, Jacopo Mondi wrote:
>>>> Hi Laurent,
>>>
>>> Sorry, I forgot to add my tag
>>> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>>>
>>>> On Fri, Nov 08, 2019 at 09:42:27PM +0200, Laurent Pinchart wrote:
>>>>> 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.
>>
>> For clarification - I fully agree that removing -Wno-unused-parameter is
>> a good thing. I'm very much a -Wall -Wextra -Werror kind of guy. Fix all
>> the warnings, before someone else has to, and don't cause warnings for
>> other people.
>>
>> In terms of style, I am not fond of changing the function prototype, so
>> here's a last ditch effort of suggesting an alternate Unused()
>> definition type based on templates which I think should always work?
>> (Because the argument is used, but optimised out)
>>
>>
>> From: https://stackoverflow.com/a/39746834/3660427
>>
>> template <typename T>
>> void Unused(T&& /*No name*/) { /*Empty*/ }
>>
>> void func(int i)
>> {
>>    Unused(i); // Silent warning for unused variable
>> }
>>
>>
>> And that's purely because I believe it's cleaner to remove the Unused()
>> line when you want to then /use/ that variable rather than modify the
>> function prototype.
>>
>> But - I'll not nack this patch based on that.
>>
>> It's a shame that adding the name in via comments adds too many *'s though:
>>
>>> -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*/)
>>
>> I fear the /*device*/ reduces readability (IMO), but if you believe this
>> is the best way to solve the topic, then:
>>
>> Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> The part that bothers me with the Unused() function above is that C++
> provides a standard way to state a parameter is unused, and using a

Can you reference the C++ standard where this is mentioned please so it
can easily be found (/I can easily find it).


> different method seems a bit of a hack to me. There's also a small risk
> that new compiler versions could still produce a warning if they get too
> clever about our hacks :-S

Seems like worrying about something that hasn't happened to me, but like
I said, I'm just giving my view - You can push this patch as you wish.


> If we want not to comment out parameter names we could use
> __attribute__((__unused__)), but I think that would be even less
> readable.

That's easily wrapped in a macro just like the kernel does.

--
Kieran



> 
>>>>>> 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.
>>>>
>>>> I think the priority here is to make sure applications do not get
>>>> warnings when building with -Wunused-parameters, so I'm finw with
>>>> either removing the parameter's name completely from the function
>>>> definition or commenting it out, which I like less but mostly because
>>>> of how it looks, so I'm fine with both, as long as we make sure
>>>> applications do not get warnings...
>>>>
>>>>>>>> Fix an additional checkstyle.py error while at it.
>>>>>>>>
>>>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@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_++;
>

Hi Kieran,

On Wed, Nov 20, 2019 at 08:58:47AM +0000, Kieran Bingham wrote:
> On 19/11/2019 22:25, Laurent Pinchart wrote:
> > On Tue, Nov 19, 2019 at 01:35:41PM +0000, Kieran Bingham wrote:
> >> On 18/11/2019 21:53, Jacopo Mondi wrote:
> >>> On Sat, Nov 09, 2019 at 11:31:35AM +0100, Jacopo Mondi wrote:
> >>>> Hi Laurent,
> >>>
> >>> Sorry, I forgot to add my tag
> >>> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> >>>
> >>>> On Fri, Nov 08, 2019 at 09:42:27PM +0200, Laurent Pinchart wrote:
> >>>>> 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.
> >>
> >> For clarification - I fully agree that removing -Wno-unused-parameter is
> >> a good thing. I'm very much a -Wall -Wextra -Werror kind of guy. Fix all
> >> the warnings, before someone else has to, and don't cause warnings for
> >> other people.
> >>
> >> In terms of style, I am not fond of changing the function prototype, so
> >> here's a last ditch effort of suggesting an alternate Unused()
> >> definition type based on templates which I think should always work?
> >> (Because the argument is used, but optimised out)
> >>
> >>
> >> From: https://stackoverflow.com/a/39746834/3660427
> >>
> >> template <typename T>
> >> void Unused(T&& /*No name*/) { /*Empty*/ }
> >>
> >> void func(int i)
> >> {
> >>    Unused(i); // Silent warning for unused variable
> >> }
> >>
> >>
> >> And that's purely because I believe it's cleaner to remove the Unused()
> >> line when you want to then /use/ that variable rather than modify the
> >> function prototype.
> >>
> >> But - I'll not nack this patch based on that.
> >>
> >> It's a shame that adding the name in via comments adds too many *'s though:
> >>
> >>> -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*/)
> >>
> >> I fear the /*device*/ reduces readability (IMO), but if you believe this
> >> is the best way to solve the topic, then:
> >>
> >> Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 
> > The part that bothers me with the Unused() function above is that C++
> > provides a standard way to state a parameter is unused, and using a
> 
> Can you reference the C++ standard where this is mentioned please so it
> can easily be found (/I can easily find it).

Looks like I was wrong. The only reference I can find in the C++
standard is in section 11.4.1.6 of
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4659.pdf:

"[ Note: Unused parameters need not be named."

> > different method seems a bit of a hack to me. There's also a small risk
> > that new compiler versions could still produce a warning if they get too
> > clever about our hacks :-S
> 
> Seems like worrying about something that hasn't happened to me, but like
> I said, I'm just giving my view - You can push this patch as you wish.

I agree with you, it may be an unnecessary worry.

> > If we want not to comment out parameter names we could use
> > __attribute__((__unused__)), but I think that would be even less
> > readable.
> 
> That's easily wrapped in a macro just like the kernel does.

Given that the C++ standard doesn't specify unnamed parameters to be the
official way to silence unused parameters warnings, I think this is
acceptable.

Where should we then put this ? Would a compiler.h header make sense ?
Or we could use __attribute__((__unused__)) explicitly, or
[[gnu:unused]]. This would in any case (even with a macro) result in
longer lines than commenting out the parameter name :-) I still prefer
commenting it out for this reason, but if the majority disagrees with
me, I'll give up :-)

> >>>>>> 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.
> >>>>
> >>>> I think the priority here is to make sure applications do not get
> >>>> warnings when building with -Wunused-parameters, so I'm finw with
> >>>> either removing the parameter's name completely from the function
> >>>> definition or commenting it out, which I like less but mostly because
> >>>> of how it looks, so I'm fine with both, as long as we make sure
> >>>> applications do not get warnings...
> >>>>
> >>>>>>>> Fix an additional checkstyle.py error while at it.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@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_++;