[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