[PATCH] libcamera: formatting: Avoid spaces in for loops without expression
Stanislaw Gruszka
stanislaw.gruszka at linux.intel.com
Wed Feb 26 09:12:28 CET 2025
Hi,
On Tue, Feb 25, 2025 at 11:14:07PM +0000, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2025-02-25 21:22:03)
> > Hi Milan,
> >
> > Thank you for the patch.
> >
> > On Tue, Feb 25, 2025 at 04:28:06PM +0100, Milan Zamazal wrote:
> > > The clang formatter removes spaces in places of empty expressions in for
> > > loops. For example, it changes
> > >
> > > for (init; condition; )
> > >
> > > to
> > >
> > > for (init; condition;)
> > >
> > > libcamera currently uses both the styles and we should use only one of
> > > them for consistency. Since there is apparently no option to override
> > > the formatter behavior (see
> > > https://clang.llvm.org/docs/ClangFormatStyleOptions.html), let's remove
> > > the extra spaces to make the formatter happy.
> > >
> > > Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> >
> > It's not my favourite style, but I'm OK with it given the inconvenience
> > of making it a clang-format exception.
>
> I remember we discussed this quite some time ago, but I'm fine with
> making the formatter happy - that makes all our lives easier.
BTW I had formatter issue when editing src/libcamera/v4l2_device.cpp
It get me changes like this:
@@ -482,11 +482,12 @@ int V4L2Device::setFrameStartEnabled(bool enable)
if (frameStartEnabled_ == enable)
return 0;
- struct v4l2_event_subscription event{};
+ struct v4l2_event_subscription event {
+ };
event.type = V4L2_EVENT_FRAME_SYNC;
unsigned long request = enable ? VIDIOC_SUBSCRIBE_EVENT
- : VIDIOC_UNSUBSCRIBE_EVENT;
+ : VIDIOC_UNSUBSCRIBE_EVENT;
int ret = ioctl(request, &event);
if (enable && ret)
return ret;
@@ -828,7 +829,8 @@ void V4L2Device::updateControls(ControlList *ctrls,
*/
void V4L2Device::eventAvailable()
{
- struct v4l2_event event{};
+ struct v4l2_event event {
+ };
int ret = ioctl(VIDIOC_DQEVENT, &event);
if (ret < 0) {
LOG(V4L2, Error)
@@ -850,11 +852,7 @@ void V4L2Device::eventAvailable()
static const std::map<uint32_t, ColorSpace> v4l2ToColorSpace = {
{ V4L2_COLORSPACE_RAW, ColorSpace::Raw },
- { V4L2_COLORSPACE_SRGB, {
- ColorSpace::Primaries::Rec709,
- ColorSpace::TransferFunction::Srgb,
- ColorSpace::YcbcrEncoding::Rec601,
- ColorSpace::Range::Limited } },
+ { V4L2_COLORSPACE_SRGB, { ColorSpace::Primaries::Rec709, ColorSpace::TransferFunction::Srgb, ColorSpace::YcbcrEncoding::Rec601, ColorSpace::Range::Limited } },
{ V4L2_COLORSPACE_JPEG, ColorSpace::Sycc },
{ V4L2_COLORSPACE_SMPTE170M, ColorSpace::Smpte170m },
{ V4L2_COLORSPACE_REC709, ColorSpace::Rec709 },
What looks fine for me. I think I can post this patch.
But some other auto-format changes does not look that good.
For example in src/libcamera/v4l2_subdevice.cpp
I have bunch of hunks like this
{ MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE, {
- .name = "RGB444_2X8_PADHI_BE",
- .code = MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE,
- .type = MediaBusFormatInfo::Type::Image,
- .bitsPerPixel = 16,
- .colourEncoding = PixelFormatInfo::ColourEncodingRGB,
- } },
+ .name = "RGB444_2X8_PADHI_BE",
+ .code = MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE,
+ .type = MediaBusFormatInfo::Type::Image,
+ .bitsPerPixel = 16,
+ .colourEncoding = PixelFormatInfo::ColourEncodingRGB,
+ } },
and one such change
- for (unsigned int index = 0; ; index++) {
+ for (unsigned int index = 0;; index++) {
what looks less convenient for me and perhaps is
candidate for changing .clang-format rule if possible.
Regards
Stanislaw
More information about the libcamera-devel
mailing list