[libcamera-devel] [PATCH/RFC] meson: Remove -Wno-unused-parameter
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Oct 28 11:35:44 CET 2019
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 at ideasonboard.com>
> > ---
> > meson.build | 1 -
> > src/android/camera3_hal.cpp | 9 +++++----
> > src/android/camera_device.cpp | 4 ++--
> > src/android/camera_proxy.cpp | 4 ++--
> > src/cam/main.cpp | 2 +-
> > src/ipa/ipa_vimc.cpp | 10 +++++-----
> > src/ipa/rkisp1/rkisp1.cpp | 2 +-
> > src/libcamera/device_enumerator_udev.cpp | 2 +-
> > src/libcamera/ipc_unixsocket.cpp | 2 +-
> > src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
> > src/libcamera/pipeline/rkisp1/timeline.cpp | 2 +-
> > src/libcamera/pipeline/uvcvideo.cpp | 2 +-
> > src/libcamera/pipeline/vimc.cpp | 4 ++--
> > src/libcamera/process.cpp | 2 +-
> > src/libcamera/proxy/ipa_proxy_linux.cpp | 12 ++++++------
> > src/libcamera/v4l2_videodevice.cpp | 2 +-
> > src/qcam/main.cpp | 2 +-
> > test/camera/capture.cpp | 2 +-
> > test/libtest/test.h | 4 ++--
> > test/log/log_process.cpp | 3 ++-
> > test/message.cpp | 2 +-
> > test/process/process_test.cpp | 3 ++-
> > test/timer-thread.cpp | 2 +-
> > test/timer.cpp | 2 +-
> > 25 files changed, 43 insertions(+), 41 deletions(-)
> >
> > diff --git a/meson.build b/meson.build
> > index 72ad7c8b493b..19a921a8ba6a 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -31,7 +31,6 @@ if cc.has_header_symbol('stdlib.h', 'secure_getenv', prefix : '#define _GNU_SOUR
> > endif
> >
> > common_arguments = [
> > - '-Wno-unused-parameter',
> > '-include', 'config.h',
> > ]
> >
> > diff --git a/src/android/camera3_hal.cpp b/src/android/camera3_hal.cpp
> > index 8d2629ca356c..a7f470172583 100644
> > --- a/src/android/camera3_hal.cpp
> > +++ b/src/android/camera3_hal.cpp
> > @@ -31,18 +31,19 @@ static int hal_get_camera_info(int id, struct camera_info *info)
> > return cameraManager.getCameraInfo(id, info);
> > }
> >
> > -static int hal_set_callbacks(const camera_module_callbacks_t *callbacks)
> > +static int hal_set_callbacks(const camera_module_callbacks_t * /*callbacks*/)
> > {
> > return 0;
> > }
> >
> > -static int hal_open_legacy(const struct hw_module_t *module, const char *id,
> > - uint32_t halVersion, struct hw_device_t **device)
> > +static int hal_open_legacy(const struct hw_module_t * /*module*/,
> > + const char * /*id*/, uint32_t /*halVersion*/,
> > + struct hw_device_t ** /*device*/)
> > {
> > return -ENOSYS;
> > }
> >
> > -static int hal_set_torch_mode(const char *camera_id, bool enabled)
> > +static int hal_set_torch_mode(const char * /*camera_id*/, bool /*enabled*/)
> > {
> > return -ENOSYS;
> > }
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index c7c9b3fd1724..b0ba1cf0a921 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -49,7 +49,7 @@ CameraDevice::Camera3RequestDescriptor::~Camera3RequestDescriptor()
> > * to the framework using the designated callbacks.
> > */
> >
> > -CameraDevice::CameraDevice(unsigned int id, const std::shared_ptr<Camera> &camera)
> > +CameraDevice::CameraDevice(unsigned int /*id*/, const std::shared_ptr<Camera> &camera)
> > : running_(false), camera_(camera), staticMetadata_(nullptr)
> > {
> > camera_->requestCompleted.connect(this, &CameraDevice::requestComplete);
> > @@ -875,7 +875,7 @@ void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream)
> > /*
> > * Produce a set of fixed result metadata.
> > */
> > -std::unique_ptr<CameraMetadata> CameraDevice::getResultMetadata(int frame_number,
> > +std::unique_ptr<CameraMetadata> CameraDevice::getResultMetadata(int /*frame_number*/,
> > int64_t timestamp)
> > {
> > /*
> > diff --git a/src/android/camera_proxy.cpp b/src/android/camera_proxy.cpp
> > index 4f5c0a024903..c9f98c784eec 100644
> > --- a/src/android/camera_proxy.cpp
> > +++ b/src/android/camera_proxy.cpp
> > @@ -79,11 +79,11 @@ static int hal_dev_process_capture_request(const struct camera3_device *dev,
> > return proxy->processCaptureRequest(request);
> > }
> >
> > -static void hal_dev_dump(const struct camera3_device *dev, int fd)
> > +static void hal_dev_dump(const struct camera3_device * /*dev*/, int /*fd*/)
> > {
> > }
> >
> > -static int hal_dev_flush(const struct camera3_device *dev)
> > +static int hal_dev_flush(const struct camera3_device * /*dev*/)
> > {
> > return 0;
> > }
> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> > index 9d99f5587cbb..ceb0efeff045 100644
> > --- a/src/cam/main.cpp
> > +++ b/src/cam/main.cpp
> > @@ -326,7 +326,7 @@ int CamApp::run()
> > return 0;
> > }
> >
> > -void signalHandler(int signal)
> > +void signalHandler(int /*signal*/)
> > {
> > std::cout << "Exiting" << std::endl;
> > CamApp::instance()->quit();
> > diff --git a/src/ipa/ipa_vimc.cpp b/src/ipa/ipa_vimc.cpp
> > index 63d578b4e2aa..16c7e0732d40 100644
> > --- a/src/ipa/ipa_vimc.cpp
> > +++ b/src/ipa/ipa_vimc.cpp
> > @@ -30,11 +30,11 @@ public:
> > ~IPAVimc();
> >
> > int init() override;
> > - void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> > - const std::map<unsigned int, ControlInfoMap> &entityControls) override {}
> > - void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
> > - void unmapBuffers(const std::vector<unsigned int> &ids) override {}
> > - void processEvent(const IPAOperationData &event) override {}
> > + void configure(const std::map<unsigned int, IPAStream> & /*streamConfig*/,
> > + const std::map<unsigned int, ControlInfoMap> & /*entityControls*/) override {}
> > + void mapBuffers(const std::vector<IPABuffer> & /*buffers*/) override {}
> > + void unmapBuffers(const std::vector<unsigned int> & /*ids*/) override {}
> > + void processEvent(const IPAOperationData & /*event*/) override {}
> >
> > private:
> > void initTrace();
> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> > index 9a13f5c7df17..84f270791340 100644
> > --- a/src/ipa/rkisp1/rkisp1.cpp
> > +++ b/src/ipa/rkisp1/rkisp1.cpp
> > @@ -61,7 +61,7 @@ private:
> > uint32_t maxGain_;
> > };
> >
> > -void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,
> > +void IPARkISP1::configure(const std::map<unsigned int, IPAStream> & /*streamConfig*/,
> > const std::map<unsigned int, ControlInfoMap> &entityControls)
> > {
> > if (entityControls.empty())
> > diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
> > index b2c5fd221dcd..6e0fbbd8c70c 100644
> > --- a/src/libcamera/device_enumerator_udev.cpp
> > +++ b/src/libcamera/device_enumerator_udev.cpp
> > @@ -309,7 +309,7 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)
> > return 0;
> > }
> >
> > -void DeviceEnumeratorUdev::udevNotify(EventNotifier *notifier)
> > +void DeviceEnumeratorUdev::udevNotify(EventNotifier * /*notifier*/)
> > {
> > struct udev_device *dev = udev_monitor_receive_device(monitor_);
> > std::string action(udev_device_get_action(dev));
> > diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp
> > index def08eef00f8..eb594268dd6c 100644
> > --- a/src/libcamera/ipc_unixsocket.cpp
> > +++ b/src/libcamera/ipc_unixsocket.cpp
> > @@ -306,7 +306,7 @@ int IPCUnixSocket::recvData(void *buffer, size_t length,
> > return 0;
> > }
> >
> > -void IPCUnixSocket::dataNotifier(EventNotifier *notifier)
> > +void IPCUnixSocket::dataNotifier(EventNotifier * /*notifier*/)
> > {
> > int ret;
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 8d3ad568d16e..84356646b736 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -700,7 +700,7 @@ error:
> > }
> >
> > int PipelineHandlerIPU3::freeBuffers(Camera *camera,
> > - const std::set<Stream *> &streams)
> > + const std::set<Stream *> & /*streams*/)
> > {
> > IPU3CameraData *data = cameraData(camera);
> >
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 7a28b03b8d38..aed060bada70 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -703,7 +703,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,
> > }
> >
> > int PipelineHandlerRkISP1::freeBuffers(Camera *camera,
> > - const std::set<Stream *> &streams)
> > + const std::set<Stream *> & /*streams*/)
> > {
> > RkISP1CameraData *data = cameraData(camera);
> >
> > diff --git a/src/libcamera/pipeline/rkisp1/timeline.cpp b/src/libcamera/pipeline/rkisp1/timeline.cpp
> > index f6c6434d7b53..59e6de78c79d 100644
> > --- a/src/libcamera/pipeline/rkisp1/timeline.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/timeline.cpp
> > @@ -204,7 +204,7 @@ void Timeline::updateDeadline()
> > timer_.start(deadline);
> > }
> >
> > -void Timeline::timeout(Timer *timer)
> > +void Timeline::timeout(Timer * /*timer*/)
> > {
> > utils::time_point now = std::chrono::steady_clock::now();
> >
> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> > index fae0ffc4de30..94464c7c7f0c 100644
> > --- a/src/libcamera/pipeline/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo.cpp
> > @@ -208,7 +208,7 @@ int PipelineHandlerUVC::allocateBuffers(Camera *camera,
> > }
> >
> > int PipelineHandlerUVC::freeBuffers(Camera *camera,
> > - const std::set<Stream *> &streams)
> > + const std::set<Stream *> & /*streams*/)
> > {
> > UVCCameraData *data = cameraData(camera);
> > return data->video_->releaseBuffers();
> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> > index c16ae4cb76b5..7e325469f178 100644
> > --- a/src/libcamera/pipeline/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc.cpp
> > @@ -166,7 +166,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)
> > {
> > }
> >
> > -CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
> > +CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera * /*camera*/,
> > const StreamRoles &roles)
> > {
> > CameraConfiguration *config = new VimcCameraConfiguration();
> > @@ -274,7 +274,7 @@ int PipelineHandlerVimc::allocateBuffers(Camera *camera,
> > }
> >
> > int PipelineHandlerVimc::freeBuffers(Camera *camera,
> > - const std::set<Stream *> &streams)
> > + const std::set<Stream *> & /*streams*/)
> > {
> > VimcCameraData *data = cameraData(camera);
> > return data->video_->releaseBuffers();
> > diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
> > index 3b4d0f10da67..44ebfec178fe 100644
> > --- a/src/libcamera/process.cpp
> > +++ b/src/libcamera/process.cpp
> > @@ -89,7 +89,7 @@ void sigact(int signal, siginfo_t *info, void *ucontext)
> >
> > } /* namespace */
> >
> > -void ProcessManager::sighandler(EventNotifier *notifier)
> > +void ProcessManager::sighandler(EventNotifier * /*notifier*/)
> > {
> > char data;
> > ssize_t ret = read(pipe_[0], &data, sizeof(data));
> > diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
> > index 4e6fa6899e07..d4ccf5112cd7 100644
> > --- a/src/libcamera/proxy/ipa_proxy_linux.cpp
> > +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
> > @@ -27,11 +27,11 @@ public:
> > ~IPAProxyLinux();
> >
> > int init() override { return 0; }
> > - void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> > - const std::map<unsigned int, ControlInfoMap> &entityControls) override {}
> > - void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
> > - void unmapBuffers(const std::vector<unsigned int> &ids) override {}
> > - void processEvent(const IPAOperationData &event) override {}
> > + void configure(const std::map<unsigned int, IPAStream> & /*streamConfig*/,
> > + const std::map<unsigned int, ControlInfoMap> & /*entityControls*/) override {}
> > + void mapBuffers(const std::vector<IPABuffer> & /*buffers*/) override {}
> > + void unmapBuffers(const std::vector<unsigned int> & /*ids*/) override {}
> > + void processEvent(const IPAOperationData & /*event*/) override {}
> >
> > private:
> > void readyRead(IPCUnixSocket *ipc);
> > @@ -86,7 +86,7 @@ IPAProxyLinux::~IPAProxyLinux()
> > delete socket_;
> > }
> >
> > -void IPAProxyLinux::readyRead(IPCUnixSocket *ipc)
> > +void IPAProxyLinux::readyRead(IPCUnixSocket * /*ipc*/)
> > {
> > }
> >
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index 208ab54199b1..87d810d7cfa4 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -1141,7 +1141,7 @@ Buffer *V4L2VideoDevice::dequeueBuffer()
> > * For Capture video devices the Buffer will contain valid data.
> > * For Output video devices the Buffer can be considered empty.
> > */
> > -void V4L2VideoDevice::bufferAvailable(EventNotifier *notifier)
> > +void V4L2VideoDevice::bufferAvailable(EventNotifier * /*notifier*/)
> > {
> > Buffer *buffer = dequeueBuffer();
> > if (!buffer)
> > diff --git a/src/qcam/main.cpp b/src/qcam/main.cpp
> > index a7ff5c52663b..d8cfc1c3d76b 100644
> > --- a/src/qcam/main.cpp
> > +++ b/src/qcam/main.cpp
> > @@ -17,7 +17,7 @@
> > #include "../cam/options.h"
> > #include "qt_event_dispatcher.h"
> >
> > -void signalHandler(int signal)
> > +void signalHandler(int /*signal*/)
> > {
> > std::cout << "Exiting" << std::endl;
> > qApp->quit();
> > diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> > index 791ccad15f70..a5fd0a641705 100644
> > --- a/test/camera/capture.cpp
> > +++ b/test/camera/capture.cpp
> > @@ -19,7 +19,7 @@ protected:
> > unsigned int completeBuffersCount_;
> > unsigned int completeRequestsCount_;
> >
> > - void bufferComplete(Request *request, Buffer *buffer)
> > + void bufferComplete(Request * /*request*/, Buffer *buffer)
> > {
> > if (buffer->status() != Buffer::BufferSuccess)
> > return;
> > diff --git a/test/libtest/test.h b/test/libtest/test.h
> > index ec08bf97c03d..193d7aa99f38 100644
> > --- a/test/libtest/test.h
> > +++ b/test/libtest/test.h
> > @@ -26,11 +26,11 @@ public:
> > protected:
> > virtual int init() { return 0; }
> > virtual int run() = 0;
> > - virtual void cleanup() { }
> > + virtual void cleanup() {}
> > };
> >
> > #define TEST_REGISTER(klass) \
> > -int main(int argc, char *argv[]) \
> > +int main(int /*argc*/, char * /*argv*/[]) \
> > { \
> > return klass().execute(); \
> > }
> > diff --git a/test/log/log_process.cpp b/test/log/log_process.cpp
> > index 2df4aa43713c..fa5639dde7cb 100644
> > --- a/test/log/log_process.cpp
> > +++ b/test/log/log_process.cpp
> > @@ -123,7 +123,8 @@ protected:
> > }
> >
> > private:
> > - void procFinished(Process *proc, enum Process::ExitStatus exitStatus, int exitCode)
> > + void procFinished(Process * /*proc*/,
> > + enum Process::ExitStatus exitStatus, int exitCode)
> > {
> > exitStatus_ = exitStatus;
> > exitCode_ = exitCode;
> > diff --git a/test/message.cpp b/test/message.cpp
> > index 3775c30a20b3..3e2659c836e3 100644
> > --- a/test/message.cpp
> > +++ b/test/message.cpp
> > @@ -35,7 +35,7 @@ public:
> > void reset() { status_ = NoMessage; }
> >
> > protected:
> > - void message(Message *msg)
> > + void message(Message * /*msg*/)
> > {
> > if (thread() != Thread::current())
> > status_ = InvalidThread;
> > diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp
> > index 7e7b3c2c8bf3..2ba3efd08457 100644
> > --- a/test/process/process_test.cpp
> > +++ b/test/process/process_test.cpp
> > @@ -75,7 +75,8 @@ protected:
> > }
> >
> > private:
> > - void procFinished(Process *proc, enum Process::ExitStatus exitStatus, int exitCode)
> > + void procFinished(Process * /*proc*/,
> > + enum Process::ExitStatus exitStatus, int exitCode)
> > {
> > exitStatus_ = exitStatus;
> > exitCode_ = exitCode;
> > diff --git a/test/timer-thread.cpp b/test/timer-thread.cpp
> > index 32853b4e80ef..55b7abcbbeab 100644
> > --- a/test/timer-thread.cpp
> > +++ b/test/timer-thread.cpp
> > @@ -39,7 +39,7 @@ public:
> > }
> >
> > private:
> > - void timeoutHandler(Timer *timer)
> > + void timeoutHandler(Timer * /*timer*/)
> > {
> > timeout_ = true;
> > }
> > diff --git a/test/timer.cpp b/test/timer.cpp
> > index 2bdb006edccb..03df03aa8d69 100644
> > --- a/test/timer.cpp
> > +++ b/test/timer.cpp
> > @@ -56,7 +56,7 @@ public:
> > }
> >
> > private:
> > - void timeoutHandler(Timer *timer)
> > + void timeoutHandler(Timer * /*timer*/)
> > {
> > expiration_ = std::chrono::steady_clock::now();
> > count_++;
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list