[libcamera-devel] [RFC PATCH] libcamera: Rename 'method' to 'function'

Laurent Pinchart laurent.pinchart at ideasonboard.com
Fri Aug 6 12:10:23 CEST 2021


Hi Kieran,

On Fri, Aug 06, 2021 at 10:57:50AM +0100, Kieran Bingham wrote:
> On 05/08/2021 17:35, Laurent Pinchart wrote:
> > Usage of 'method' to refer to member functions comes from Java. The C++
> > standard uses the term 'function' only. Replace 'method' with 'function'
> > or 'member function' through the whole code base and documentation.
> 
> Ouch ;-)
> 
> I think method is widely known as a 'function of a class' ... but indeed
> that's also more correctly known as a member function I think.
> 
> So - using 'function' over 'method' is perhaps more correct for C++.
> 
> 
> I won't nitpick over this for specific reviews, as long as it doesn't
> break compilation, and checkstyle is happy, I'm sure you've dealt with
> line lengths and reflows accordingly.

Paul went through the patch and found a typo in a comment :-)

> Acked-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> I do wonder if you might consider adding the rule to checkstyle though ...

Yes, but I think we should rename BoundMethod first in that case.
Otherwise there will be too many false positives.

Work on BoundMethod is on my todo list, I'd like to investigate if the
custom implementation could be replaced (or at least simplified) with
usage of C++17 features such as std::apply, but that's not high
priority.

> But - perhaps that might just be a generic 's/method/function/' on the
> whole patch content to see if it picks up any changes?
> 
> Essentially it's a custom spell check correction ... hrm ...
> 
> > The BoundMethod and Object::invokeMethod() are left as-is here, and will
> > be addressed separately.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > I've carried this in my tree for too long, and it's very annoying to
> > rebase as you can imagine, so I thought I'd sent it out to get it
> > accepted or rejected, but out of its current limbo.
> > ---
> >  Documentation/coding-style.rst                |   2 +-
> >  Documentation/docs.rst                        |   2 +-
> >  .../guides/application-developer.rst          |   4 +-
> >  Documentation/guides/ipa.rst                  |  10 +-
> >  Documentation/guides/pipeline-handler.rst     | 102 +++++++++---------
> >  include/libcamera/ipa/core.mojom              |   2 +-
> >  include/libcamera/ipa/raspberrypi.mojom       |  31 +++---
> >  src/android/camera_device.cpp                 |   2 +-
> >  src/android/jpeg/exif.cpp                     |   2 +-
> >  src/ipa/ipu3/ipu3.cpp                         |   2 +-
> >  src/ipa/libipa/camera_sensor_helper.cpp       |   2 +-
> >  src/ipa/raspberrypi/cam_helper.cpp            |   4 +-
> >  src/ipa/raspberrypi/cam_helper.hpp            |  14 +--
> >  src/ipa/raspberrypi/controller/controller.hpp |   4 +-
> >  src/ipa/raspberrypi/controller/rpi/alsc.hpp   |   2 +-
> >  .../raspberrypi/controller/rpi/sharpen.cpp    |   2 +-
> >  src/ipa/raspberrypi/md_parser.hpp             |   4 +-
> >  src/ipa/raspberrypi/raspberrypi.cpp           |   2 +-
> >  src/libcamera/base/event_dispatcher_poll.cpp  |   2 +-
> >  src/libcamera/base/log.cpp                    |  10 +-
> >  src/libcamera/base/message.cpp                |   6 +-
> >  src/libcamera/base/object.cpp                 |  22 ++--
> >  src/libcamera/base/semaphore.cpp              |  16 +--
> >  src/libcamera/base/signal.cpp                 |   2 +-
> >  src/libcamera/base/thread.cpp                 |  40 +++----
> >  src/libcamera/byte_stream_buffer.cpp          |   9 +-
> >  src/libcamera/camera.cpp                      |  24 ++---
> >  src/libcamera/camera_manager.cpp              |   2 +-
> >  src/libcamera/camera_sensor.cpp               |  24 ++---
> >  src/libcamera/control_serializer.cpp          |   6 +-
> >  src/libcamera/control_validator.cpp           |   2 +-
> >  src/libcamera/controls.cpp                    |  20 ++--
> >  src/libcamera/device_enumerator.cpp           |   2 +-
> >  src/libcamera/file_descriptor.cpp             |  14 +--
> >  src/libcamera/formats.cpp                     |   2 +-
> >  src/libcamera/framebuffer.cpp                 |  10 +-
> >  src/libcamera/framebuffer_allocator.cpp       |   4 +-
> >  src/libcamera/geometry.cpp                    |   2 +-
> >  src/libcamera/ipa_interface.cpp               |  14 +--
> >  src/libcamera/ipa_manager.cpp                 |  11 +-
> >  src/libcamera/ipa_module.cpp                  |  12 +--
> >  src/libcamera/ipa_proxy.cpp                   |   4 +-
> >  src/libcamera/ipc_unixsocket.cpp              |  22 ++--
> >  src/libcamera/media_device.cpp                |   4 +-
> >  src/libcamera/pipeline/ipu3/imgu.cpp          |   4 +-
> >  src/libcamera/pipeline_handler.cpp            |  58 +++++-----
> >  src/libcamera/process.cpp                     |  10 +-
> >  src/libcamera/request.cpp                     |  11 +-
> >  src/libcamera/stream.cpp                      |  17 +--
> >  src/libcamera/v4l2_device.cpp                 |  20 ++--
> >  src/libcamera/v4l2_videodevice.cpp            |  16 +--
> >  test/media_device/media_device_link_test.cpp  |   2 +-
> >  test/signal.cpp                               |   4 +-
> >  53 files changed, 313 insertions(+), 308 deletions(-)

[snip]

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list