[PATCH v2 00/20] Remove unused includes
Milan Zamazal
mzamazal at redhat.com
Mon Sep 2 18:01:17 CEST 2024
Hi Laurent,
Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:
> Hi Milan,
>
> Thank you for the series. It looks like you went through a rabbit hole
> :-)
>
> On Fri, Aug 30, 2024 at 05:26:57PM +0200, Milan Zamazal wrote:
>> There is quite a lot of include’s that are actually unused. They don’t
>> cause much trouble but they should still be removed in order to:
>>
>> - keep order,
>> - avoid LSP warnings when working with the code,
>
> Which LSP server do you use ? clangd ?
Yes.
> I assume it's based on clang-format for coding formatting, so that
> should be fine as that's we use in checkstyle.py too.
>
>> - to remove unneeded extra lines.
>>
>> I have identified the unused includes by inspecting warnings issued by
>> LSP. LSP understands the difference between direct and indirect
>> imports. I have inspected its reports in some cases and it seems to be
>> correct most of the time so I trust it and haven’t inspected everything,
>> which would be waste of resources.
>
> I wasted some resources, sorry :-)
Thank you.
>> I have noticed only two problems:
>>
>> - LSP reports in my environment unused <chrono> includes despite of
>> presence of
>>
>> using std::literals::chrono_literals;
>>
>> or
>>
>> using std::chrono_literals;
>>
>> (these two should be probably unified but this is out of scope of this
>> series). This happens also in a trivial code and is most likely a
>> bug.
>>
>> - LSP may report unnecessary inclusion of
>> "libcamera/internal/framebuffer.h" although it is actually needed to
>> be able to access something from FrameBuffer::Private.
>>
>> It’s interesting that in both the cases LSP reports the corresponding
>> problems when the supposedly unused include is removed.
>>
>> I’m also not sure whether header include removal can affect Doxygen
>> output in some way. I supposed it doesn’t but if it does then the
>> patches must be inspected for such cases.
>
> I'm not aware of any issue there.
>
>> I haven’t removed includes of "FOO.h" in FOO.cpp files even if the
>> include is unused (which happens when FOO.cpp contains only
>> documentation).
>
> Good choice. Include the .h helps making sure it's self-contained.
>
>> My LSP based autoformatter reformatted some of the modified files. Many
>> of the suggested changes look right so I kept most of them (and helped
>> the autoformatter to do a better thing in some cases), in separate
>> commits, rather than discarding them. It’s annoying when editing a
>> source file and the autoformatter changes the code elsewhere; if the
>> autoformatter is wrong anywhere then the autoformatting rules should be
>> fixed if possible.
>
> Some of the changes look right and good, some are more in neutral
> territory to me, but quite a few I don't like. Last time I looked at
> clang-format there I couldn't get it to match our coding style
> perfectly, but new options may have been added. If so, I agree it would
> be best to bring it as close as possible to how we want code to be
> formatted. There will always be exceptions though.
OK. For now, I simply reverted the unwanted changes and kept those that
you didn't object strongly enough. We can work on improvements later,
as needed.
>> I have inspected many source files but not all of them. Although the
>> cleanup is most likely a bit incomplete, it should still make things
>> significantly better.
>>
>> I’m sorry for another long patch series but I tried to split the patches
>> in order to make them reasonably sized (and easy to review!) and by
>> possible areas of responsibility.
>
> You did well, thank you for that.
>
>> Changes in v2:
>> - Add a missing include in tests.
>>
>> Milan Zamazal (20):
>> tests: Add a missing iostream include
>> libcamera: ipu3: Remove unused includes
>> libcamera: ipu3: Replace wrong include
>> libcamera: ipu3: Formatting improvements
>> libcamera: rkisp1: Remove unused includes
>> libcamera: rkisp1: Formatting improvements
>> libcamera: libipa: Remove unused includes
>> libcamera: uvcvideo: Remove unused includes
>> libcamera: uvcvideo: Formatting improvement
>> libcamera: v4l2: Remove unused includes
>> libcamera: v4l2: Formatting improvements
>> libcamera: v4l2_subdevice: Formatting improvements
>> libcamera: v4l2: Fix indirect include
>> libcamera: ipa: Remove unused includes
>> libcamera: libcamera: Remove unused includes
>> libcamera: libcamera: Add missing includes
>> libcamera: libcamera: Formatting improvements
>> libcamera: includes: Add missing includes
>> libcamera: includes: Remove unused includes
>> libcamera: includes: Formatting improvements
>>
>> include/libcamera/base/event_dispatcher.h | 2 -
>> include/libcamera/base/log.h | 23 +-
>> include/libcamera/base/memfd.h | 2 -
>> include/libcamera/base/signal.h | 1 -
>> include/libcamera/base/span.h | 41 +-
>> include/libcamera/base/timer.h | 1 -
>> include/libcamera/base/utils.h | 3 +-
>> include/libcamera/framebuffer.h | 1 -
>> include/libcamera/internal/camera_manager.h | 5 +-
>> include/libcamera/internal/camera_sensor.h | 1 -
>> .../internal/device_enumerator_sysfs.h | 1 -
>> .../libcamera/internal/dma_buf_allocator.h | 2 -
>> include/libcamera/internal/formats.h | 1 -
>> .../libcamera/internal/ipa_data_serializer.h | 11 +-
>> include/libcamera/internal/ipa_proxy.h | 2 -
>> .../libcamera/internal/ipc_pipe_unixsocket.h | 1 -
>> include/libcamera/internal/media_device.h | 1 -
>> include/libcamera/internal/pipeline_handler.h | 3 -
>> include/libcamera/internal/request.h | 1 +
>> .../libcamera/internal/shared_mem_object.h | 1 -
>> include/libcamera/ipa/ipa_interface.h | 5 -
>> include/libcamera/logging.h | 2 +
>> include/libcamera/pixel_format.h | 1 -
>> include/libcamera/request.h | 1 -
>> include/libcamera/stream.h | 1 -
>> include/libcamera/transform.h | 2 -
>> src/ipa/ipu3/algorithms/af.cpp | 3 -
>> src/ipa/ipu3/algorithms/agc.cpp | 32 +-
>> src/ipa/ipu3/algorithms/blc.cpp | 6 +-
>> src/ipa/ipu3/ipu3.cpp | 16 +-
>> src/ipa/libipa/histogram.h | 1 -
>> src/ipa/libipa/matrix.h | 1 -
>> src/ipa/libipa/matrix_interpolator.cpp | 7 -
>> src/ipa/libipa/matrix_interpolator.h | 2 -
>> src/ipa/libipa/pwl.cpp | 2 -
>> src/ipa/libipa/pwl.h | 3 -
>> src/ipa/libipa/vector.h | 2 -
>> src/ipa/rkisp1/algorithms/agc.h | 1 -
>> src/ipa/rkisp1/algorithms/awb.cpp | 20 +-
>> src/ipa/rkisp1/algorithms/ccm.cpp | 4 -
>> src/ipa/rkisp1/algorithms/dpf.cpp | 6 +-
>> src/ipa/rkisp1/rkisp1.cpp | 22 +-
>> src/ipa/rkisp1/utils.h | 1 -
>> src/libcamera/base/event_dispatcher_poll.cpp | 4 +-
>> src/libcamera/camera.cpp | 6 +-
>> src/libcamera/controls.cpp | 32 +-
>> .../converter/converter_v4l2_m2m.cpp | 1 -
>> src/libcamera/formats.cpp | 3 +-
>> src/libcamera/ipa_data_serializer.cpp | 97 +-
>> src/libcamera/ipa_module.cpp | 16 +-
>> src/libcamera/ipa_proxy.cpp | 1 -
>> src/libcamera/orientation.cpp | 17 +-
>> src/libcamera/pipeline/ipu3/frames.cpp | 3 +-
>> src/libcamera/pipeline/ipu3/ipu3.cpp | 41 +-
>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 -
>> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 6 +-
>> src/libcamera/pipeline_handler.cpp | 6 +-
>> src/libcamera/process.cpp | 8 +-
>> src/libcamera/sensor/camera_sensor.cpp | 8 +-
>> src/libcamera/shared_mem_object.cpp | 5 +-
>> src/libcamera/stream.cpp | 8 +-
>> src/libcamera/v4l2_device.cpp | 19 +-
>> src/libcamera/v4l2_subdevice.cpp | 1311 +++++++++--------
>> src/libcamera/v4l2_videodevice.cpp | 37 +-
>> src/v4l2/v4l2_camera.h | 1 -
>> src/v4l2/v4l2_camera_proxy.cpp | 46 +-
>> src/v4l2/v4l2_compat.cpp | 20 +-
>> src/v4l2/v4l2_compat_manager.cpp | 5 +-
>> .../generated_serializer_test.cpp | 1 +
>> 69 files changed, 967 insertions(+), 982 deletions(-)
More information about the libcamera-devel
mailing list