[coreboot] What is the best way to treat warnings reported by checkpatch.pl

Vadim Bendebury vbendeb at chromium.org
Thu Jul 28 22:40:23 CEST 2016


I have not read the entire thread, but just in case it was not
mentioned before:  even when the structure elements come together very
snugly on their own, without __packed(), it still is required if the
structure comes over a wire or from a file, etc. as __packed()
guarantees that there is no unaligned accesses.

Also, what seems to be missing in checkpatch.pl (or is ti there, I
just don't know about it?) is the ability to suppress a warning for
just a certain line of code, so that when the style deviations are
necessary, they are clearly marked/explained.

-v


On Thu, Jul 28, 2016 at 1:14 PM, Julius Werner <jwerner at chromium.org> wrote:
>> The point is, that without ((packed)) there is no guarantee that gcc
>> won't choose a different alignment over what you and I think would make
>> sense.
>
> In practice it is very predictable and there should be no sane reason
> to do it otherwise. Like with many of the other "it's not guaranteed
> in the standard but GCC and Clang have been doing it this way since
> forever" issues where being standards-compliant would incur a serious
> performance or readability hit, I'd advocate being pragmatic. Everyone
> knows the standard is stupid, but we still need to generate the
> binaries we want somehow. (And, of course, __attribute__((packed)) is
> also a GCC extension anyway.)
>
>> What other changes to checkpatch are needed?
>
> I'd really suggest that we stick with the upstream checkpatch.pl and
> use command line flags to tune it to coreboot (submitting patches for
> new flags or cleaner detection upstream if necessary). The tool gets
> updated frequently and we'd quickly fall out of sync with that if we
> don't keep a clean copy that can just be uprevved regularly. Also,
> this is what the Chromium tree does so maintaining a different fork of
> checkpatch.pl for upstream coreboot would just make lives harder for
> everyone working on Chromium (or, more likely, would just make them
> commit with --no-verify all the time). Working together to improve a
> common upstream would make all our lives easier.
>
> --
> coreboot mailing list: coreboot at coreboot.org
> https://www.coreboot.org/mailman/listinfo/coreboot



More information about the coreboot mailing list