I am not sure if this happened in coreboot upstream: on some other projects checkpatch.pl was recently updated to the latest Linux kernel version, and some more stupid warnings/errors started popping out.
There is a fine controlling checkpatch.pl behavior, .checkpatch.conf in the coreboot root directory. That file can be modified to quiet down checkpatch.pl complaints which are not applicable to coreboot. --vb On Mon, Jul 25, 2016 at 11:46 PM, Zeh, Werner <[email protected]> wrote: > Hi community. > > > > I recently ran into a blocked commit due to warnings reported by > checkpatch.pl. > > In this particular case I wanted to add the “ATSR” structure to the DMAR > tables. To do that, I need to define a new structure > > which reflects the per specification needed layout of this table entry. > > I did that in the same style how other structures are defined in the same > file (src/arch/x86/include/arch/acpi.h): > > > > typedef struct dmar_atsr_entry { > > u16 type; > > u16 length; > > u8 flags; > > u8 reserved; > > u16 segment; > > } __attribute__ ((packed)) dmar_atsr_entry_t; > > > > This change resulted in two warnings caused by util/lint/checkpatch.pl > when I was trying to do a git commit: > > > > WARNING: do not add new typedefs > > #34: FILE: src/arch/x86/include/arch/acpi.h:288: > > +typedef struct dmar_atsr_entry { > > > > WARNING: __packed is preferred over __attribute__((packed)) > > #40: FILE: src/arch/x86/include/arch/acpi.h:294: > > +} __attribute__ ((packed)) dmar_atsr_entry_t; > > > > So the first warning tells me that I should not use “typedef” in my code. > > > > The second one tells me not to use __attribute__ ((packed) but instead > __packed! > > If one take a closer look to the coreboot tree, one will not find a macro > definition called __packed at all. > > > > After a short discussion on IRC with Stefan it was clear that although we > have this checkpatch.pl script in the tree and the git hook “pre-commit” > > activates it, the script does not perfectly match current coreboot coding > style. > > > > So in the end there needs to be a change somewhere: > > 1. We can adapt the script to match current coreboot coding style > > 2. We can start to refactor the coreboot tree to match the demands of > checkpatch.pl > > 3. We can align only new patches to the demands of the script > > 4. We can show warnings from the script but prevent them from > aborting the git commit > > > > While 1. would be the best, straight forward approach for the project, it > might cause high effort for a person who knows the code base very well. > > 2. will lead to a lot of changes in the tree while the reason/goal is > still questionable (at least to me). > > 3. will lead to different code style all over the tree (even in the same > file) and in the end will result in fragmented code. I personally would not > like it. > > 4. can be a short term solution while we are seeking for the best way to > deal with this issue. > > > > There may be other ways to go, too, which I have overseen now. > > > > I just want to start this discussion officially as this is not the first > time I ran into issues with checkpatch.pl. > > What do you think about it, what would be the best way to handle this case? > > > > Your ideas and thoughts are appreciated. > > > > Werner > > -- > coreboot mailing list: [email protected] > https://www.coreboot.org/mailman/listinfo/coreboot >
-- coreboot mailing list: [email protected] https://www.coreboot.org/mailman/listinfo/coreboot
