Change in flashrom[master]: [WIP] Add .clang-format to help with patch formatting

[David Hendricks (Code Review)]

Hello Vadim Bendebury,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/flashrom/+/38673

to review the following change.

Change subject: [WIP] Add .clang-format to help with patch formatting
......................................................................

[WIP] Add .clang-format to help with patch formatting

This is based on CB:38208 by Vadim, but with a few added changes.
Here is the original commit message.

This is a copy from the coreboot tree, could be tweaked to better
match the flashrom tree if there are any differences.

Once this file is in place a script could be deployed which would
format only new/changed lines in git patches.

Change-Id: I1fe22809c4a2d73fa02f4de55290d990a9750b86
Signed-off-by: Vadim Bendebury 
Signed-off-by: David Hendricks 
---
A .clang-format
1 file changed, 21 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/38673/1

diff --git a/.clang-format b/.clang-format
new file mode 100644
index 0000000..45c116b
--- /dev/null
+++ b/.clang-format
@@ -0,0 +1,21 @@
+BasedOnStyle:					LLVM
+Language:					Cpp
+IndentWidth:					8
+UseTab:						Always
+BreakBeforeBraces:				Linux
+AllowShortIfStatementsOnASingleLine:		false
+IndentCaseLabels:				false
+SortIncludes:					false
+ContinuationIndentWidth:			8
+ColumnLimit:					112
+AlwaysBreakBeforeMultilineStrings:		true
+AllowShortLoopsOnASingleLine:			false
+AllowShortFunctionsOnASingleLine:		false
+AlignEscapedNewlinesLeft:			false
+AlignTrailingComments:				true
+AllowAllParametersOfDeclarationOnNextLine:	false
+AlignAfterOpenBracket:				true
+SpaceAfterCStyleCast:				false
+MaxEmptyLinesToKeep:				2
+BreakBeforeBinaryOperators:			NonAssignment
+BreakStringLiterals:				false

[David Hendricks (Code Review)]

Hello build bot (Jenkins), Vadim Bendebury,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/38673

to look at the new patch set (#2).

Change subject: [WIP] Add .clang-format to help with patch formatting
......................................................................

[WIP] Add .clang-format to help with patch formatting

This is based on CB:38208 by Vadim, but with a few added changes.
Here is the original commit message.

This is a copy from the coreboot tree, could be tweaked to better
match the flashrom tree if there are any differences.

Once this file is in place a script could be deployed which would
format only new/changed lines in git patches.

Change-Id: I1fe22809c4a2d73fa02f4de55290d990a9750b86
Signed-off-by: Vadim Bendebury 
Signed-off-by: David Hendricks 
---
A .clang-format
1 file changed, 21 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/38673/2

[Vadim Bendebury (Code Review)]

Vadim Bendebury has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/38673 )

Change subject: [WIP] Add .clang-format to help with patch formatting
......................................................................

Patch Set 3: Code-Review+1

lgtm - I did not even look at the differences, but IMO any clang format config is better than no config :)

[David Hendricks (Code Review)]

Hello build bot (Jenkins), Vadim Bendebury,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/38673

to look at the new patch set (#4).

Change subject: [WIP] Add .clang-format to help with patch formatting
......................................................................

[WIP] Add .clang-format to help with patch formatting

This is based on CB:38208 but with a few added items. It's just a
brain dump that has not been thoroughly tested. If it's useful and
gets polished up then it will be squashed into Vadim's version.

For now, this patch has the sources and headers modified as per the
current patchset's .clang-format to illustrate its effects. We might
choose not to modify the sources at the same time the config file is
merged.

Change-Id: I1fe22809c4a2d73fa02f4de55290d990a9750b86
Signed-off-by: Vadim Bendebury 
Signed-off-by: David Hendricks 
---
A .clang-format
M 82802ab.c
M amd_imc.c
M at45db.c
M atahpt.c
M atapromise.c
M atavia.c
M bitbang_spi.c
M board_enable.c
M buspirate_spi.c
M cbtable.c
M ch341a_spi.c
M chipdrivers.h
M chipset_enable.c
M cli_classic.c
M cli_common.c
M cli_output.c
M coreboot_tables.h
M custom_baud.c
M dediprog.c
M developerbox_spi.c
M digilent_spi.c
M dmi.c
M drkaiser.c
M dummyflasher.c
M edi.c
M edi.h
M en29lv640b.c
M ene.h
M flash.h
M flashchips.c
M flashchips.h
M flashrom.c
M fmap.c
M fmap.h
M ft2232_spi.c
M gfxnvidia.c
M helpers.c
M hwaccess.c
M hwaccess.h
M i2c_helper_linux.c
M ich_descriptors.c
M ich_descriptors.h
M ichspi.c
M internal.c
M it8212.c
M it85spi.c
M it87spi.c
M jedec.c
M jlink_spi.c
M layout.c
M layout.h
M libflashrom.c
M libflashrom.h
M linux_mtd.c
M linux_spi.c
M lspcon_i2c_spi.c
M mcp6x_spi.c
M mstarddc_spi.c
M ni845x_spi.c
M nic3com.c
M nicintel.c
M nicintel_eeprom.c
M nicintel_spi.c
M nicnatsemi.c
M nicrealtek.c
M ogp_spi.c
M opaque.c
M os.h
M pcidev.c
M physmap.c
M pickit2_spi.c
M platform.h
M pony_spi.c
M print.c
M print_wiki.c
M processor_enable.c
M programmer.c
M programmer.h
M raiden_debug_spi.c
M rayer_spi.c
M realtek_mst_i2c_spi.c
M satamv.c
M satasii.c
M sb600spi.c
M serial.c
M serprog.c
M serprog.h
M sfdp.c
M spi.c
M spi.h
M spi25.c
M spi25_statusreg.c
M spi95.c
M sst28sf040.c
M sst49lfxxxc.c
M sst_fwhub.c
M stlinkv3_spi.c
M stm50.c
M udelay.c
M usb_device.c
M usb_device.h
M usbblaster_spi.c
M usbdev.c
M w29ee011.c
M w39.c
M wbsio_spi.c
107 files changed, 6,737 insertions(+), 7,547 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/38673/4

[David Hendricks (Code Review)]

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/38673 )

Change subject: [WIP] Add .clang-format to help with patch formatting
......................................................................

Patch Set 4: Code-Review-2

(3 comments)

I was experimenting with this some more and figured it might help move things along if people can actually see the result on gerrit. There are parts that need more tweaking, so hopefully somebody with more clang-format experience can offer suggestions to fix remaining issues.

https://review.coreboot.org/c/flashrom/+/38673/4/buspirate_spi.c 
File buspirate_spi.c:

https://review.coreboot.org/c/flashrom/+/38673/4/buspirate_spi.c@148 
PS4, Line 148: static const struct buspirate_speeds spispeeds[] = { { "30k", 0x0 }, { "125k", 0x1 }, { "250k", 0x2 },
clang-format seems to have weird rules regarding lists. The only workaround I found for this is to set ColumnLimit to 0, but that has obvious issues with line length.

https://review.coreboot.org/c/flashrom/+/38673/4/chipset_enable.c 
File chipset_enable.c:

https://review.coreboot.org/c/flashrom/+/38673/4/chipset_enable.c@659 
PS4, Line 659: 	case CHIPSET_300_SERIES_CANNON_POINT: boot_straps = boot_straps_pch8_lp; break;
We use case labels inconsistently - some are on their own line and some are condensed so that the entire statement is on a single line.

I suspect we'll want to use the non-condensed format throughout flashrom, i.e. AllowShortCaseLabelsOnASingleLine: false

https://review.coreboot.org/c/flashrom/+/38673/4/dediprog.c 
File dediprog.c:

https://review.coreboot.org/c/flashrom/+/38673/4/dediprog.c@60 
PS4, Line 60: enum dediprog_leds {
Alignment of assignments is another area where clang-format seems lacking. We generally don't force alignment of variable assignments, however when the assignments form a list of allowable values such as these enums it's preferable to align the assignments.

[David Hendricks (Code Review)]

Hello build bot (Jenkins), Vadim Bendebury,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/38673

to look at the new patch set (#5).

Change subject: [WIP] Add .clang-format to help with patch formatting
......................................................................

[WIP] Add .clang-format to help with patch formatting

This is based on CB:38208 but with a few added items. It's just a
brain dump that has not been thoroughly tested. If it's useful and
gets polished up then it will be squashed into Vadim's version.

For now, this patch has the sources and headers modified as per the
current patchset's .clang-format to illustrate its effects. We might
choose not to modify the sources at the same time the config file is
merged.

See also CB:42556 which uses uncrustify instead of clang-format.

Change-Id: I1fe22809c4a2d73fa02f4de55290d990a9750b86
Signed-off-by: Vadim Bendebury 
Signed-off-by: David Hendricks 
---
A .clang-format
M 82802ab.c
M amd_imc.c
M at45db.c
M atahpt.c
M atapromise.c
M atavia.c
M bitbang_spi.c
M board_enable.c
M buspirate_spi.c
M cbtable.c
M ch341a_spi.c
M chipdrivers.h
M chipset_enable.c
M cli_classic.c
M cli_common.c
M cli_output.c
M coreboot_tables.h
M custom_baud.c
M dediprog.c
M developerbox_spi.c
M digilent_spi.c
M dmi.c
M drkaiser.c
M dummyflasher.c
M edi.c
M edi.h
M en29lv640b.c
M ene.h
M flash.h
M flashchips.c
M flashchips.h
M flashrom.c
M fmap.c
M fmap.h
M ft2232_spi.c
M gfxnvidia.c
M helpers.c
M hwaccess.c
M hwaccess.h
M i2c_helper_linux.c
M ich_descriptors.c
M ich_descriptors.h
M ichspi.c
M internal.c
M it8212.c
M it85spi.c
M it87spi.c
M jedec.c
M jlink_spi.c
M layout.c
M layout.h
M libflashrom.c
M libflashrom.h
M linux_mtd.c
M linux_spi.c
M lspcon_i2c_spi.c
M mcp6x_spi.c
M mstarddc_spi.c
M ni845x_spi.c
M nic3com.c
M nicintel.c
M nicintel_eeprom.c
M nicintel_spi.c
M nicnatsemi.c
M nicrealtek.c
M ogp_spi.c
M opaque.c
M os.h
M pcidev.c
M physmap.c
M pickit2_spi.c
M platform.h
M pony_spi.c
M print.c
M print_wiki.c
M processor_enable.c
M programmer.c
M programmer.h
M raiden_debug_spi.c
M rayer_spi.c
M realtek_mst_i2c_spi.c
M satamv.c
M satasii.c
M sb600spi.c
M serial.c
M serprog.c
M serprog.h
M sfdp.c
M spi.c
M spi.h
M spi25.c
M spi25_statusreg.c
M spi95.c
M sst28sf040.c
M sst49lfxxxc.c
M sst_fwhub.c
M stlinkv3_spi.c
M stm50.c
M udelay.c
M usb_device.c
M usb_device.h
M usbblaster_spi.c
M usbdev.c
M w29ee011.c
M w39.c
M wbsio_spi.c
107 files changed, 6,737 insertions(+), 7,547 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/38673/5

[David Hendricks (Code Review)]

David Hendricks has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/38673 )

Change subject: [WIP] Add .clang-format to help with patch formatting
......................................................................

Abandoned

Abandoning in favor of uncrustify: CB:42556

[David Hendricks (Code Review)]

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/38673 )

Change subject: [WIP] Add .clang-format to help with patch formatting
......................................................................

Patch Set 5:

(3 comments)

https://review.coreboot.org/c/flashrom/+/38673/5/atapromise.c 
File atapromise.c:

https://review.coreboot.org/c/flashrom/+/38673/5/atapromise.c@145 
PS5, Line 145: 	msg_pwarn(
Not sure why clang-format aligned it this way... Maybe BreakStringLiterals is needed? But in this case the '\n' embedded in the string itself might show up in a weird place.

https://review.coreboot.org/c/flashrom/+/38673/4/board_enable.c 
File board_enable.c:

https://review.coreboot.org/c/flashrom/+/38673/4/board_enable.c@305 
PS4, Line 305: 			msg_pdbg(
More instances of weird alignment of strings as arguments to print functions.

https://review.coreboot.org/c/flashrom/+/38673/4/ich_descriptors.h 
File ich_descriptors.h:

https://review.coreboot.org/c/flashrom/+/38673/4/ich_descriptors.h@118 
PS4, Line 118: 			uint32_t : 17, freq_read : 3, fastread : 1, freq_fastread : 3, freq_write : 3,
AlignConsecutiveBitFields might work in these cases?