diff mbox series

scripts/bpf_doc: add __main__ judgement before main code

Message ID 20231130145746.23621-1-2023002089@link.tyut.edu.cn (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series scripts/bpf_doc: add __main__ judgement before main code | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
netdev/tree_selection success Not a local patch, async
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-10 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-11 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-9 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-18 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-llvm-16 / build / build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-llvm-16 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-llvm-16 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-16 / veristat
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-16 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-llvm-16 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-16

Commit Message

Hu Haowen Nov. 30, 2023, 2:57 p.m. UTC 
When doing Python programming it is a nice convention to insert the if
statement `if __name__ == "__main__":` before any main code that does
actual functionalities to ensure the code will be executed only as a
script rather than as an imported module.  Hence attach the missing
judgement to bpf_doc.py.

Signed-off-by: Hu Haowen <2023002089@link.tyut.edu.cn>
---
 scripts/bpf_doc.py | 81 +++++++++++++++++++++++-----------------------
 1 file changed, 41 insertions(+), 40 deletions(-)

Comments

Daniel Borkmann Dec. 1, 2023, 3:46 p.m. UTC | #1 
Hi Hu,

On 11/30/23 3:57 PM, Hu Haowen wrote:
> When doing Python programming it is a nice convention to insert the if
> statement `if __name__ == "__main__":` before any main code that does
> actual functionalities to ensure the code will be executed only as a
> script rather than as an imported module.  Hence attach the missing
> judgement to bpf_doc.py.
> 
> Signed-off-by: Hu Haowen <2023002089@link.tyut.edu.cn>

Thanks for the patch. What's the concrete value of this one? Do you plan
to distribute the bpf_docs.py outside of the kernel tree? If it's not
needed feels somewhat too much churn/marginal value.

>   scripts/bpf_doc.py | 81 +++++++++++++++++++++++-----------------------
>   1 file changed, 41 insertions(+), 40 deletions(-)
> 
> diff --git a/scripts/bpf_doc.py b/scripts/bpf_doc.py
> index 61b7dddedc46..af2a87d97832 100755
> --- a/scripts/bpf_doc.py
> +++ b/scripts/bpf_doc.py
> @@ -851,43 +851,44 @@ class PrinterHelpers(Printer):
>   
>   ###############################################################################
>   
> -# If script is launched from scripts/ from kernel tree and can access
> -# ../include/uapi/linux/bpf.h, use it as a default name for the file to parse,
> -# otherwise the --filename argument will be required from the command line.
> -script = os.path.abspath(sys.argv[0])
> -linuxRoot = os.path.dirname(os.path.dirname(script))
> -bpfh = os.path.join(linuxRoot, 'include/uapi/linux/bpf.h')
> -
> -printers = {
> -        'helpers': PrinterHelpersRST,
> -        'syscall': PrinterSyscallRST,
> -}
> -
> -argParser = argparse.ArgumentParser(description="""
> -Parse eBPF header file and generate documentation for the eBPF API.
> -The RST-formatted output produced can be turned into a manual page with the
> -rst2man utility.
> -""")
> -argParser.add_argument('--header', action='store_true',
> -                       help='generate C header file')
> -if (os.path.isfile(bpfh)):
> -    argParser.add_argument('--filename', help='path to include/uapi/linux/bpf.h',
> -                           default=bpfh)
> -else:
> -    argParser.add_argument('--filename', help='path to include/uapi/linux/bpf.h')
> -argParser.add_argument('target', nargs='?', default='helpers',
> -                       choices=printers.keys(), help='eBPF API target')
> -args = argParser.parse_args()
> -
> -# Parse file.
> -headerParser = HeaderParser(args.filename)
> -headerParser.run()
> -
> -# Print formatted output to standard output.
> -if args.header:
> -    if args.target != 'helpers':
> -        raise NotImplementedError('Only helpers header generation is supported')
> -    printer = PrinterHelpers(headerParser)
> -else:
> -    printer = printers[args.target](headerParser)
> -printer.print_all()
> +if __name__ == "__main__":
> +    # If script is launched from scripts/ from kernel tree and can access
> +    # ../include/uapi/linux/bpf.h, use it as a default name for the file to parse,
> +    # otherwise the --filename argument will be required from the command line.
> +    script = os.path.abspath(sys.argv[0])
> +    linuxRoot = os.path.dirname(os.path.dirname(script))
> +    bpfh = os.path.join(linuxRoot, 'include/uapi/linux/bpf.h')
> +
> +    printers = {
> +            'helpers': PrinterHelpersRST,
> +            'syscall': PrinterSyscallRST,
> +    }
> +
> +    argParser = argparse.ArgumentParser(description="""
> +    Parse eBPF header file and generate documentation for the eBPF API.
> +    The RST-formatted output produced can be turned into a manual page with the
> +    rst2man utility.
> +    """)
> +    argParser.add_argument('--header', action='store_true',
> +                           help='generate C header file')
> +    if (os.path.isfile(bpfh)):
> +        argParser.add_argument('--filename', help='path to include/uapi/linux/bpf.h',
> +                               default=bpfh)
> +    else:
> +        argParser.add_argument('--filename', help='path to include/uapi/linux/bpf.h')
> +    argParser.add_argument('target', nargs='?', default='helpers',
> +                           choices=printers.keys(), help='eBPF API target')
> +    args = argParser.parse_args()
> +
> +    # Parse file.
> +    headerParser = HeaderParser(args.filename)
> +    headerParser.run()
> +
> +    # Print formatted output to standard output.
> +    if args.header:
> +        if args.target != 'helpers':
> +            raise NotImplementedError('Only helpers header generation is supported')
> +        printer = PrinterHelpers(headerParser)
> +    else:
> +        printer = printers[args.target](headerParser)
> +    printer.print_all()
>
Hu Haowen Dec. 2, 2023, 5:56 a.m. UTC | #2 
On 2023/12/1 23:46, Daniel Borkmann wrote:
> Hi Hu,
>
> On 11/30/23 3:57 PM, Hu Haowen wrote:
>> When doing Python programming it is a nice convention to insert the if
>> statement `if __name__ == "__main__":` before any main code that does
>> actual functionalities to ensure the code will be executed only as a
>> script rather than as an imported module.  Hence attach the missing
>> judgement to bpf_doc.py.
>>
>> Signed-off-by: Hu Haowen <2023002089@link.tyut.edu.cn>
>
> Thanks for the patch. What's the concrete value of this one? Do you plan
> to distribute the bpf_docs.py outside of the kernel tree? If it's not
> needed feels somewhat too much churn/marginal value.
>

Just noticed that the main code section does not follow the common rule
within the Python programming convention that any main code is ought to
be specified underneath the __name__ == "__main__" judgement to keep it
away from being executed when imported as a module wrongly. Hence I gave
this suggested change to both abide by this good convention and enhance
the main code's safety. After all it is recommended to have this change
carried out.

Thanks,
Hu Haowen


>>   scripts/bpf_doc.py | 81 +++++++++++++++++++++++-----------------------
>>   1 file changed, 41 insertions(+), 40 deletions(-)
>>
>> diff --git a/scripts/bpf_doc.py b/scripts/bpf_doc.py
>> index 61b7dddedc46..af2a87d97832 100755
>> --- a/scripts/bpf_doc.py
>> +++ b/scripts/bpf_doc.py
>> @@ -851,43 +851,44 @@ class PrinterHelpers(Printer):
>> ###############################################################################
>>   -# If script is launched from scripts/ from kernel tree and can access
>> -# ../include/uapi/linux/bpf.h, use it as a default name for the file 
>> to parse,
>> -# otherwise the --filename argument will be required from the 
>> command line.
>> -script = os.path.abspath(sys.argv[0])
>> -linuxRoot = os.path.dirname(os.path.dirname(script))
>> -bpfh = os.path.join(linuxRoot, 'include/uapi/linux/bpf.h')
>> -
>> -printers = {
>> -        'helpers': PrinterHelpersRST,
>> -        'syscall': PrinterSyscallRST,
>> -}
>> -
>> -argParser = argparse.ArgumentParser(description="""
>> -Parse eBPF header file and generate documentation for the eBPF API.
>> -The RST-formatted output produced can be turned into a manual page 
>> with the
>> -rst2man utility.
>> -""")
>> -argParser.add_argument('--header', action='store_true',
>> -                       help='generate C header file')
>> -if (os.path.isfile(bpfh)):
>> -    argParser.add_argument('--filename', help='path to 
>> include/uapi/linux/bpf.h',
>> -                           default=bpfh)
>> -else:
>> -    argParser.add_argument('--filename', help='path to 
>> include/uapi/linux/bpf.h')
>> -argParser.add_argument('target', nargs='?', default='helpers',
>> -                       choices=printers.keys(), help='eBPF API target')
>> -args = argParser.parse_args()
>> -
>> -# Parse file.
>> -headerParser = HeaderParser(args.filename)
>> -headerParser.run()
>> -
>> -# Print formatted output to standard output.
>> -if args.header:
>> -    if args.target != 'helpers':
>> -        raise NotImplementedError('Only helpers header generation is 
>> supported')
>> -    printer = PrinterHelpers(headerParser)
>> -else:
>> -    printer = printers[args.target](headerParser)
>> -printer.print_all()
>> +if __name__ == "__main__":
>> +    # If script is launched from scripts/ from kernel tree and can 
>> access
>> +    # ../include/uapi/linux/bpf.h, use it as a default name for the 
>> file to parse,
>> +    # otherwise the --filename argument will be required from the 
>> command line.
>> +    script = os.path.abspath(sys.argv[0])
>> +    linuxRoot = os.path.dirname(os.path.dirname(script))
>> +    bpfh = os.path.join(linuxRoot, 'include/uapi/linux/bpf.h')
>> +
>> +    printers = {
>> +            'helpers': PrinterHelpersRST,
>> +            'syscall': PrinterSyscallRST,
>> +    }
>> +
>> +    argParser = argparse.ArgumentParser(description="""
>> +    Parse eBPF header file and generate documentation for the eBPF API.
>> +    The RST-formatted output produced can be turned into a manual 
>> page with the
>> +    rst2man utility.
>> +    """)
>> +    argParser.add_argument('--header', action='store_true',
>> +                           help='generate C header file')
>> +    if (os.path.isfile(bpfh)):
>> +        argParser.add_argument('--filename', help='path to 
>> include/uapi/linux/bpf.h',
>> +                               default=bpfh)
>> +    else:
>> +        argParser.add_argument('--filename', help='path to 
>> include/uapi/linux/bpf.h')
>> +    argParser.add_argument('target', nargs='?', default='helpers',
>> +                           choices=printers.keys(), help='eBPF API 
>> target')
>> +    args = argParser.parse_args()
>> +
>> +    # Parse file.
>> +    headerParser = HeaderParser(args.filename)
>> +    headerParser.run()
>> +
>> +    # Print formatted output to standard output.
>> +    if args.header:
>> +        if args.target != 'helpers':
>> +            raise NotImplementedError('Only helpers header 
>> generation is supported')
>> +        printer = PrinterHelpers(headerParser)
>> +    else:
>> +        printer = printers[args.target](headerParser)
>> +    printer.print_all()
>>
>
>
diff mbox series

Patch

diff --git a/scripts/bpf_doc.py b/scripts/bpf_doc.py
index 61b7dddedc46..af2a87d97832 100755
--- a/scripts/bpf_doc.py
+++ b/scripts/bpf_doc.py
@@ -851,43 +851,44 @@  class PrinterHelpers(Printer):
 
 ###############################################################################
 
-# If script is launched from scripts/ from kernel tree and can access
-# ../include/uapi/linux/bpf.h, use it as a default name for the file to parse,
-# otherwise the --filename argument will be required from the command line.
-script = os.path.abspath(sys.argv[0])
-linuxRoot = os.path.dirname(os.path.dirname(script))
-bpfh = os.path.join(linuxRoot, 'include/uapi/linux/bpf.h')
-
-printers = {
-        'helpers': PrinterHelpersRST,
-        'syscall': PrinterSyscallRST,
-}
-
-argParser = argparse.ArgumentParser(description="""
-Parse eBPF header file and generate documentation for the eBPF API.
-The RST-formatted output produced can be turned into a manual page with the
-rst2man utility.
-""")
-argParser.add_argument('--header', action='store_true',
-                       help='generate C header file')
-if (os.path.isfile(bpfh)):
-    argParser.add_argument('--filename', help='path to include/uapi/linux/bpf.h',
-                           default=bpfh)
-else:
-    argParser.add_argument('--filename', help='path to include/uapi/linux/bpf.h')
-argParser.add_argument('target', nargs='?', default='helpers',
-                       choices=printers.keys(), help='eBPF API target')
-args = argParser.parse_args()
-
-# Parse file.
-headerParser = HeaderParser(args.filename)
-headerParser.run()
-
-# Print formatted output to standard output.
-if args.header:
-    if args.target != 'helpers':
-        raise NotImplementedError('Only helpers header generation is supported')
-    printer = PrinterHelpers(headerParser)
-else:
-    printer = printers[args.target](headerParser)
-printer.print_all()
+if __name__ == "__main__":
+    # If script is launched from scripts/ from kernel tree and can access
+    # ../include/uapi/linux/bpf.h, use it as a default name for the file to parse,
+    # otherwise the --filename argument will be required from the command line.
+    script = os.path.abspath(sys.argv[0])
+    linuxRoot = os.path.dirname(os.path.dirname(script))
+    bpfh = os.path.join(linuxRoot, 'include/uapi/linux/bpf.h')
+
+    printers = {
+            'helpers': PrinterHelpersRST,
+            'syscall': PrinterSyscallRST,
+    }
+
+    argParser = argparse.ArgumentParser(description="""
+    Parse eBPF header file and generate documentation for the eBPF API.
+    The RST-formatted output produced can be turned into a manual page with the
+    rst2man utility.
+    """)
+    argParser.add_argument('--header', action='store_true',
+                           help='generate C header file')
+    if (os.path.isfile(bpfh)):
+        argParser.add_argument('--filename', help='path to include/uapi/linux/bpf.h',
+                               default=bpfh)
+    else:
+        argParser.add_argument('--filename', help='path to include/uapi/linux/bpf.h')
+    argParser.add_argument('target', nargs='?', default='helpers',
+                           choices=printers.keys(), help='eBPF API target')
+    args = argParser.parse_args()
+
+    # Parse file.
+    headerParser = HeaderParser(args.filename)
+    headerParser.run()
+
+    # Print formatted output to standard output.
+    if args.header:
+        if args.target != 'helpers':
+            raise NotImplementedError('Only helpers header generation is supported')
+        printer = PrinterHelpers(headerParser)
+    else:
+        printer = printers[args.target](headerParser)
+    printer.print_all()