Closed jeremy-compostella closed 5 years ago
The following is the result of run kernel checkpatch.pl:
checkpatch.pl --terse -f --max-line-length 132 libadb/lspci.c libadb/lspci.c:1: WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 libadb/lspci.c:39: WARNING: do not add new typedefs libadb/lspci.c:41: ERROR: spaces required around that ':' (ctx:VxW) libadb/lspci.c:42: ERROR: spaces required around that ':' (ctx:VxW) libadb/lspci.c:43: ERROR: spaces required around that ':' (ctx:VxW) libadb/lspci.c:48: WARNING: do not add new typedefs libadb/lspci.c:59: WARNING: packed is preferred over attribute__((packed)) libadb/lspci.c:125: WARNING: Missing a blank line after declarations libadb/lspci.c:171: WARNING: Avoid line continuations in quoted strings total: 3 errors, 6 warnings, 178 lines checked
We can ignore the WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 and WARNING: do not add new typedefs, but fix the others.
Thanks for running that script. I included it in my process now. Note that this script may give inappropriate result considering that even thought historically the Kernelflinger project uses the Linux Kernel Coding Style there are sometimes some differences and that the right way to know what to do is to look into the current code. For instance, I was not sure about the bit field coding convention to use and here is what I did to figure it out:
I ran find . -type f -name \*.[ch] -exec grep --color -n -E ': [0-9]+;' /dev/null {} +
to identify most of the occurrences. Then I counted the number of [a-zA-Z0-9]: *[0-9]+
expressions (259) and the number of [a-zA-Z0-9] +: *[0-9]+
expressions (203) ⇒ 44% of the code base uses the Linux Kernel convention. But when I look into details most of the matches which do not follow the Linux Kernel Coding convention are import from EDKII and as thus can be discarded. So I ran find . -type f -name \*.[ch] -not -path "./libkernelflinger/protocol/*" -exec grep --color -n -E ': *[0-9]+;' /dev/null {} +
instead and then i got 85% of the code follows the Linux Kernel Convention ⇒ I should also follow this convention.
Since I worked on identifying the piece of code violating this coding convention rule, I ceased the occasion to write a patch to resolve these violations
I ran ~/bin/scripts/checkpatch.pl --terse -g HEAD~4..HEAD
and after filtering out (I modified the checkpatch.pl
) Linux Kernel specific WARNING
I am left with:
Location | Message | Justification |
---|---|---|
a68d958b:56 | Prefer using '"%s…", func' to using 'inl', this function's name, in a string | The name of the function is the same as the assembly instruction |
a68d958b:62 | Prefer using '"%s…", func' to using 'outl', this function's name, in a string | The name of the function is the same as the assembly instruction |
I also noticed that you changed the max line length in your check. The Kernelflinger project follows the 80 characters line length convention. Same as the Linux Kernel. I know that this rule has broke a lot in the last three years but this is still supposed to be the rule.
@tanminger any update on this pull request ?
@tanminger any update on this pull request ? Build failed in Celadon.
/bin/bash -c "PWD=/proc/self/cwd prebuilts/clang/host/linux-x86/clang-r353983c/bin/clang -I hardware/intel/kernelflinger/libadb/../include/libadb -I hardware/intel/kernelflinger/libadb -I out/target/product/caas/obj/STATIC_LIBRARIES/libadb-userdebug_intermediates -I out/target/product/caas/gen/STATIC_LIBRARIES/libadb-userdebug_intermediates -I libnativehelper/include_jni \$(cat out/target/product/caas/obj/STATIC_LIBRARIES/libadb-userdebug_intermediates/import_includes) -c -Wall -Wextra -Werror -mrdrnd -DAVB_AB_I_UNDERSTAND_LIBAVB_AB_IS_DEPRECATED -D__STDC_VERSION__=199901L -DARCH_X86_64=1 -DUSE_TRUSTY -DUSE_ACPIO -DUSERDEBUG -DUSE_TPM -DSOFT_FUSE -DBOARD_BOOTIMAGE_PARTITION_SIZE=31457280 -DCRASHMODE_USE_ADB -DUSE_UI -DUSE_AVB -DAVB_ENABLE_DEBUG -DUSE_SLOT -DUSB_STORAGE -DLIVE_BOOT -DRPMB_STORAGE -DRPMB_SIMULATE -DSECURE_STORAGE_EFIVAR -DDYNAMIC_PARTITIONS -DFASTBOOT_KEYBOX_PROVISION -ggdb -O3 -fstack-protector-strong -fno-strict-aliasing -fpic -fshort-wchar -mno-red-zone -mno-mmx -fno-builtin -m64 -mstackrealign -mstack-alignment=32 -ffreestanding -fno-stack-check -Wno-pointer-sign -Wno-address-of-packed-member -Wno-macro-redefined -Wno-pointer-bool-conversion -Wno-unused-const-variable -Wno-constant-conversion -Wno-unused-function -Wno-tautological-pointer-compare -Wformat -Wformat-security -D_FORTIFY_SOURCE=2 -DEFI_FUNCTION_WRAPPER -DGNU_EFI_USE_MS_ABI -fPIC -D_USING_LIBCXX -DANDROID_STRICT -Werror=int-to-pointer-cast -Werror=pointer-to-int-cast -Werror=address-of-temporary -Werror=return-type -Wno-tautological-constant-compare -Wno-tautological-type-limit-compare -Wno-tautological-unsigned-enum-zero-compare -Wno-tautological-unsigned-zero-compare -Wno-c++98-compat-extra-semi -Wno-return-std-move-in-c++11 -MD -MF out/target/product/caas/obj/STATIC_LIBRARIES/libadb-userdebug_intermediates/lspci.d -o out/target/product/caas/obj/STATIC_LIBRARIES/libadb-userdebug_intermediates/lspci.o hardware/intel/kernelflinger/libadb/lspci.c"
hardware/intel/kernelflinger/libadb/lspci.c:91:32: error: incompatible pointer types initializing 'UINTN ' (aka 'unsigned long ') with an expression of type 'enum class_fmt ' [-Werror,-Wincompatible-pointer-types]
{ .option = "-n", .variable = &class_fmt, .value = NUMERIC },
^~~~~~
hardware/intel/kernelflinger/libadb/lspci.c:92:33: error: incompatible pointer types initializing 'UINTN ' (aka 'unsigned long ') with an expression of type 'enum class_fmt ' [-Werror,-Wincompatible-pointer-types]
{ .option = "-nn", .variable = &class_fmt, .value = BOTH }
^~~~~~
2 errors generated.
09:40:32 ninja failed with: exit status 1
make: *** [run_soong_ui] Error 1
Thanks for telling me. I'll look into it tomorrow.
On Thu, Oct 10, 2019, 6:54 PM Ming Tan notifications@github.com wrote:
@tanminger https://github.com/tanminger any update on this pull request ? Build failed in Celadon.
/bin/bash -c "PWD=/proc/self/cwd prebuilts/clang/host/linux-x86/clang-r353983c/bin/clang -I hardware/intel/kernelflinger/libadb/../include/libadb -I hardware/intel/kernelflinger/libadb -I out/target/product/caas/obj/STATIC_LIBRARIES/libadb-userdebug_intermediates -I out/target/product/caas/gen/STATIC_LIBRARIES/libadb-userdebug_intermediates -I libnativehelper/include_jni $(cat out/target/product/caas/obj/STATIC_LIBRARIES/libadb-userdebug_intermediates/import_includes) -c -Wall -Wextra -Werror -mrdrnd -DAVB_AB_I_UNDERSTAND_LIBAVB_AB_IS_DEPRECATED -D__STDC_VERSION__=199901L -DARCH_X86_64=1 -DUSE_TRUSTY -DUSE_ACPIO -DUSERDEBUG -DUSE_TPM -DSOFT_FUSE -DBOARD_BOOTIMAGE_PARTITION_SIZE=31457280 -DCRASHMODE_USE_ADB -DUSE_UI -DUSE_AVB -DAVB_ENABLE_DEBUG -DUSE_SLOT -DUSB_STORAGE -DLIVE_BOOT -DRPMB_STORAGE -DRPMB_SIMULATE -DSECURE_STORAGE_EFIVAR -DDYNAMIC_PARTITIONS -DFASTBOOT_KEYBOX_PROVISION -ggdb -O3 -fstack-protector-strong -fno-strict-aliasing -fpic -fshort-wchar -mno-red-zone -mno-mmx -fno-builtin -m64 -mstackrealign -mstack-alignment=32 -ffreestanding -fno-stack-check -Wno-pointer-sign -Wno-address-of-packed-member -Wno-macro-redefined -Wno-pointer-bool-conversion -Wno-unused-const-variable -Wno-constant-conversion -Wno-unused-function -Wno-tautological-pointer-compare -Wformat -Wformat-security -D_FORTIFY_SOURCE=2 -DEFI_FUNCTION_WRAPPER -DGNU_EFI_USE_MS_ABI -fPIC -D_USING_LIBCXX -DANDROID_STRICT -Werror=int-to-pointer-cast -Werror=pointer-to-int-cast -Werror=address-of-temporary -Werror=return-type -Wno-tautological-constant-compare -Wno-tautological-type-limit-compare -Wno-tautological-unsigned-enum-zero-compare -Wno-tautological-unsigned-zero-compare -Wno-c++98-compat-extra-semi -Wno-return-std-move-in-c++11 -MD -MF out/target/product/caas/obj/STATIC_LIBRARIES/libadb-userdebug_intermediates/lspci.d -o out/target/product/caas/obj/STATIC_LIBRARIES/libadb-userdebug_intermediates/lspci.o hardware/intel/kernelflinger/libadb/lspci.c" hardware/intel/kernelflinger/libadb/lspci.c:91:32: error: incompatible pointer types initializing 'UINTN ' (aka 'unsigned long ') with an expression of type 'enum class_fmt ' [-Werror,-Wincompatible-pointer-types] { .option = "-n", .variable = &class_fmt, .value = NUMERIC }, ^
~~~~~ hardware/intel/kernelflinger/libadb/lspci.c:92:33: error: incompatible pointer types initializing 'UINTN ' (aka 'unsigned long ') with an expression of type 'enum class_fmt ' [-Werror,-Wincompatible-pointer-types] { .option = "-nn", .variable = &class_fmt, .value = BOTH } ^~~~~~ 2 errors generated. 09:40:32 ninja failed with: exit status 1 make: *** [run_soong_ui] Error 1— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/intel/kernelflinger/pull/39?email_source=notifications&email_token=AAMACBCDYLLHZ7M324YHPCLQN7MHDA5CNFSM4I5U5V5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEA6PPWY#issuecomment-540866523, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMACBBZYIMNRLPLXHVNNBDQN7MHDANCNFSM4I5U5V5A .
It turns out that when working on issue with regard to BIOS PCI/PCIe enumeration it is handy to be able to enumerate the PCI devices from Kernelflinger Crashmode.
This implementation shared some option with the legacy pciutils - lspci. this implementation uses the legacy method via I/O addresses
0xCF8
and0xCFC
which should work on any hardware.