On Wed, Jun 25 2025, Peter Maydell <peter.mayd...@linaro.org> wrote: > On Wed, 25 Jun 2025 at 10:10, Eric Auger <eric.au...@redhat.com> wrote: >> However there are other checkpatch errors besides the one you reported, in >> 52873a54ad arm/cpu: Store aa64isar0/aa64zfr0 into the idregs arrays >> ERROR: line over 90 characters >> #388: FILE: target/arm/kvm.c:225: >> + return ARM64_SYS_REG((sysreg & CP_REG_ARM64_SYSREG_OP0_MASK) >> >> CP_REG_ARM64_SYSREG_OP0_SHIFT, >> >> ERROR: line over 90 characters >> #389: FILE: target/arm/kvm.c:226: >> + (sysreg & CP_REG_ARM64_SYSREG_OP1_MASK) >> >> CP_REG_ARM64_SYSREG_OP1_SHIFT, >> >> ERROR: line over 90 characters >> #390: FILE: target/arm/kvm.c:227: >> + (sysreg & CP_REG_ARM64_SYSREG_CRN_MASK) >> >> CP_REG_ARM64_SYSREG_CRN_SHIFT, >> >> ERROR: line over 90 characters >> #391: FILE: target/arm/kvm.c:228: >> + (sysreg & CP_REG_ARM64_SYSREG_CRM_MASK) >> >> CP_REG_ARM64_SYSREG_CRM_SHIFT, >> >> ERROR: line over 90 characters >> #392: FILE: target/arm/kvm.c:229: >> + (sysreg & CP_REG_ARM64_SYSREG_OP2_MASK) >> >> CP_REG_ARM64_SYSREG_OP2_SHIFT); >> >> WARNING: line over 80 characters >> #396: FILE: target/arm/kvm.c:233: >> +static int get_host_cpu_reg(int fd, ARMHostCPUFeatures *ahcf, >> ARMIDRegisterIdx index) > > The last one of those is probably easily fixed, but note the general > remark in style.rst that it's better to have an overlong line than > one with an awkward and unreadable wrapping. (We ought to fix > checkpatch and style.rst to agree on whether to warn or error and > whether that should be at 90 chars or 100 chars, but that would require > reopening an old style argument and it's too much effort.)
I would not disagree with fixing the last one if wanted, but I think the others are more readable if we do not try to break them up. > >> 5f15ebdf3f arm/cpu: Add sysreg definitions in cpu-sysregs.h >> ERROR: Macros with complex values should be enclosed in parenthesis >> #56: FILE: target/arm/cpu-sysregs.h:21: >> +#define DEF(NAME, OP0, OP1, CRN, CRM, OP2) NAME##_IDX, >> >> ERROR: Macros with complex values should be enclosed in parenthesis >> #64: FILE: target/arm/cpu-sysregs.h:29: >> +#define DEF(NAME, OP0, OP1, CRN, CRM, OP2) \ >> + SYS_##NAME = ENCODE_ID_REG(OP0, OP1, CRN, CRM, OP2), >> >> ERROR: Macros with complex values should be enclosed in parenthesis >> #203: FILE: target/arm/cpu64.c:40: >> +#define DEF(NAME, OP0, OP1, CRN, CRM, OP2) \ >> + [NAME##_IDX] = SYS_##NAME, > > This is checkpatch not being able to cope with more complex > uses of the preprocessor; the warning only makes sense for > "acts like a function" macros. Yep, that's why I ignored it. In general, should we mention that we intentionally ignored checkpatch.pl feedback, or would that be just noise?