Patch Set 2:

Patch Set 2:

David, I'm just curious - why do you prefer uncrustify. I never heard of it, whereas clang-format seems to be a very popular mechanism for this.

That's a good question. I'm not an expert at either, but from what I've seen uncrustify has a lot more configurability that can address specific cases in flashrom that I ran across. It's been around since at least 2006 AFAICT and has fewer package dependencies. My overall impression is that clang-format does a pretty good job by default but lacks configurability that is needed to handle some cases in flashrom.

For example, clang-format combines lines in array definitions in a way that impacts a lot of flashrom code in an undesirable way such as in https://review.coreboot.org/c/flashrom/+/38673/4/buspirate_spi.c#148, or https://review.coreboot.org/c/flashrom/+/38673/5/ich_descriptors.c#227. The array elements are deliberately given their own lines which IMO is a good thing in such cases we're listing legal values and is a fairly common pattern in flashrom.

I looked around for a way to change that behavior, and it seems that clang-format will do that unless the column limit is set to 0 (not sure why that would affect that particular case) or if the array size is less than 5 elements, which seems like an arbitrary limit that the clang-format developers imposed. Adding a comment after each element in this case might work, but that seems like a silly thing to impose since these lists should be self-documenting. clang-format just doesn't handle these cases well from what I can tell.

Another example - it aligns strings used as arguments like in msg_pdbg() in an awkward way - look around in https://review.coreboot.org/c/flashrom/+/38673/5/board_enable.c for msg_pdbg() and you'll see stuff like:
msg_pdbg(
	"W836xx enter config mode worked or we were already in config mode. W836xx "
	"leave config mode had no effect.\n");

Another example is spacing in braces, i.e. { 1, 2 } vs. {1, 2}. clang-format has options for SpacesInAngles, SpacesInParentheses, SpacesInSquareBrackets, but not for braces. Or maybe I'm missing something obvious...

Long story short, I think uncrustify handles flashrom's codebase better at this time. Maybe clang-format will become more configurable in the future and we can revisit this later if people have a strong reason to prefer it.

makes perfect sense to me, I sure have seen how inflexible clang-format is (even though it is getting better). I think uncrustify should be used more widely.

View Change

To view, visit change 42556. To unsubscribe, or for help writing mail filters, visit settings.