Em Thu, 27 Feb 2025 14:30:28 +0100
Igor Mammedov <imammedo@xxxxxxxxxx> escreveu:
> On Thu, 27 Feb 2025 12:03:30 +0100
> Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> wrote:
>
> > Now that the ghes preparation patches were merged, let's add support
> > for error injection.
> >
> > On this version, HEST table got added to ACPI tables testing for aarch64 virt.
> >
> > There are also some patch reorder to help reviewers to check the changes.
> >
> > The code itself is almost identical to v4, with just a few minor nits addressed.
>
> series still has checkpatch errors 'line over 80' which are not false positive,
> it needs to be fixed
The long line warnings are at the patch adding the Python script. IMO,
all but one are false positives:
1. Long lines at patch description because of the tool output example added
inside the commit description:
ERROR: line over 90 characters
#148: FILE: scripts/arm_processor_error.py:83:
+[Hardware Error]: bus error, operation type: Generic read (type of instruction or data request cannot be determined)
ERROR: line over 90 characters
#153: FILE: scripts/arm_processor_error.py:88:
+[Hardware Error]: Program execution can be restarted reliably at the PC associated with the error.
WARNING: line over 80 characters
#170: FILE: scripts/arm_processor_error.py:105:
+[Hardware Error]: 00000000: 13 7b 04 05 01 .{...
WARNING: line over 80 characters
#174: FILE: scripts/arm_processor_error.py:109:
+[Firmware Warn]: GHES: Unhandled processor error type 0x10: micro-architectural error
ERROR: line over 90 characters
#175: FILE: scripts/arm_processor_error.py:110:
+[Firmware Warn]: GHES: Unhandled processor error type 0x14: TLB error|micro-architectural error
IMO, breaking command output at the description is a bad practice.
2. Big strings at help message:
WARNING: line over 80 characters
#261: FILE: scripts/arm_processor_error.py:196:
+ help="Power State Coordination Interface - PSCI state")
ERROR: line over 90 characters
#276: FILE: scripts/arm_processor_error.py:211:
+ help="Number of errors: 0: Single error, 1: Multiple errors, 2-65535: Error count if known")
WARNING: line over 80 characters
#278: FILE: scripts/arm_processor_error.py:213:
+ help="Error information (UEFI 2.10 tables N.18 to N.20)")
ERROR: line over 90 characters
#287: FILE: scripts/arm_processor_error.py:222:
+ help="Type of the context (0=ARM32 GPR, 5=ARM64 EL1, other values supported)")
WARNING: line over 80 characters
#1046: FILE: scripts/qmp_helper.py:442:
+ help="Marks the timestamp as precise if --timestamp is used")
WARNING: line over 80 characters
#1048: FILE: scripts/qmp_helper.py:444:
+ help=f"General Error Data Block flags: {gedb_flags_bits}")
Those might be changed if we add one variable per string to store the
help lines, at the expense of doing some code obfuscation.
I don't think doing it is a good idea.
3. Long class function names that are part of Python's standard library:
ERROR: line over 90 characters
#576: FILE: scripts/ghes_inject.py:29:
+ parser = argparse.ArgumentParser(formatter_class=argparse.ArgumentDefaultsHelpFormatter,
We can't change the big name of the argparse formatter. The only
possible fix would be to obfuscate it by doing:
format = argparse.ArgumentDefaultsHelpFormatter,
parser = argparse.ArgumentParser(formatter_class=format,
IMO this is a bad practice.
4. False-positive warning disable for pylint coding style tool:
ERROR: line over 90 characters
#805: FILE: scripts/qmp_helper.py:201:
+ data.extend(value.to_bytes(num_bytes, byteorder="little")) # pylint: disable=E1101
WARNING: line over 80 characters
#1028: FILE: scripts/qmp_helper.py:424:
+ g_gen = parser.add_argument_group("Generic Error Data") # pylint: disable=E1101
AFAIKT, those need to be at the same line for pylint to process them
properly.
5. A long name inside an indented block:
WARNING: line over 80 characters
#1109: FILE: scripts/qmp_helper.py:505:
+ value=args.gen_err_valid_bits,
Again the only solution would be to obfuscate the argument, like:
a = args.gen_err_valid_bits
value=a,
Not nice, IMHO.
Now, there is one warning that I is not a false positive, which I ended
missing:
WARNING: line over 80 characters
#1227: FILE: scripts/qmp_helper.py:623:
+ ret = self.send_cmd("qom-get", args, may_open=True, return_error=False)
I'll fix it at the next respin.
Regards,
Mauro
