Respected mentor,
Thank you for giving me the first task.
The results after running checkpatch.pl on each of the files given is as
below.
I will do my best to explain each of the information presented in the
result.
https://www.kernel.org/doc/html/latest/dev-tools/checkpatch.html -- link to
documentation used.
As per my understanding, there are 3 different levels for the information
generated.
They are explained below from highest to lowest level of strictness.
ERROR : requires immediate attention. They are most likely wrong.
WARNING : requires careful review, but not as serious issue compared to
ERROR.
CHECK : requires a look, but it is of the mildest level.
-----------------------------------------------------------------------------
RESULT (1):
./scripts/checkpatch.pl drivers/mtd/ubi/block.c
WARNING: quoted string split across lines
#354: FILE: drivers/mtd/ubi/block.c:354:
+ pr_warn("UBI: block: volume size is not a multiple of 512, "
+ "last %llu bytes are ignored!\n",
message not present in documentation.
this warning, as stated, is generated because of the quoted strings being
split across lines.
we can solve this by just keeping them in a single line.
if the last line ends with a quote and the next line also begins with a
quote, then this issue will arise.
WARNING: braces {} are not necessary for single statement blocks
#374: FILE: drivers/mtd/ubi/block.c:374:
+ if (ret) {
+ return ret;
+ }
proper way to use braces is explained in the documentation, which is
related to the message here.
single statement blocks don't require brackets, removing them will solve
the issue.
proper way to use braces is explained in the documentation.
I believe it checks whether there is only one statement written between
the braces. if condition is true, then warn the user.
WARNING: Statements terminations use 1 semicolon
#408: FILE: drivers/mtd/ubi/block.c:408:
+ goto out_free_dev;;
message not present in documentation.
using two semicolons resulted in this warning, can be solved by [ goto
out_free_dev; ].
it checks if there is another semicolon (;) immediately after one, so if
there is a ";;" anywhere, then it issues a warning.
total: 0 errors, 3 warnings, 697 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or
--fix-inplace.
drivers/mtd/ubi/block.c has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
-----------------------------------------------------------------
RESULT (2):
./scripts/checkpatch.pl drivers/net/ethernet/amazon/ena/ena_netdev.c
CHECK: multiple assignments should be avoided
#282: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:282:
+ ena_tx_ctx->num_bufs = tx_info->num_of_bufs = 1;
message not in documentation.
this warning was triggered because it devtected the assigning of values
to multiple variables in same statement using "=".
this can be solved by changing it to this instead,
ena_tx_ctx->num_bufs = 1;
tx_info->num_of_bufs = 1;
WARNING: nested (un)?likely() calls, IS_ERR already uses unlikely()
internally
#1025: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:1025:
+ if (unlikely(IS_ERR(page)))
message not in documentation.
the error could have been caused because the "IS_ERR" function already
calls the "unlikely" function internally, and we pass the "IS_ERR"
function through
"unlikely" function again, resulting in nested calls of the "unlikely"
function.
this warning can be removed if the nested calls can be avoided.
CHECK: Alignment should match open parenthesis
#1533: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:1533:
+static void ena_rx_checksum(struct ena_ring *rx_ring,
+ struct ena_com_rx_ctx *ena_rx_ctx,
message not in documentation.
this was caused because the 2nd line "struct ena_com_rx_ctx *ena_rx_ctx,"
was not aligned with the opening parenthesis of the first line "(".
aligning it should remove the message.
CHECK: Unnecessary parentheses around 'ena_rx_ctx->l3_proto ==
ENA_ETH_IO_L3_PROTO_IPV4'
#1549: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:1549:
+ if (unlikely((ena_rx_ctx->l3_proto == ENA_ETH_IO_L3_PROTO_IPV4) &&
+ (ena_rx_ctx->l3_csum_err))) {
CHECK: Unnecessary parentheses around 'ena_rx_ctx->l3_csum_err'
#1549: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:1549:
+ if (unlikely((ena_rx_ctx->l3_proto == ENA_ETH_IO_L3_PROTO_IPV4) &&
+ (ena_rx_ctx->l3_csum_err))) {
message explained in documentation.
caused by unnecessary parentheses.
message can be removed by changing to,
if (unlikely(ena_rx_ctx->l3_proto == ENA_ETH_IO_L3_PROTO_IPV4 &&
ena_rx_ctx->l3_csum_err)) {
CHECK: Unnecessary parentheses around 'ena_rx_ctx->l4_proto ==
ENA_ETH_IO_L4_PROTO_TCP'
#1561: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:1561:
+ if (likely((ena_rx_ctx->l4_proto == ENA_ETH_IO_L4_PROTO_TCP) ||
+ (ena_rx_ctx->l4_proto == ENA_ETH_IO_L4_PROTO_UDP))) {
CHECK: Unnecessary parentheses around 'ena_rx_ctx->l4_proto ==
ENA_ETH_IO_L4_PROTO_UDP'
#1561: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:1561:
+ if (likely((ena_rx_ctx->l4_proto == ENA_ETH_IO_L4_PROTO_TCP) ||
+ (ena_rx_ctx->l4_proto == ENA_ETH_IO_L4_PROTO_UDP))) {
unnecessary parentheses, as explained in the previous example.
CHECK: Blank lines aren't necessary before a close brace '}'
#1587: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:1587:
+
+}
proper usage of braces is explained in the documentation, which is
related to this message.
as stated, it is caused due to the blank line before the closing brace.
removing the blank line should close the message.
CHECK: Unnecessary parentheses around 'ena_rx_ctx->l4_proto ==
ENA_ETH_IO_L4_PROTO_TCP'
#1596: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:1596:
+ if (likely((ena_rx_ctx->l4_proto == ENA_ETH_IO_L4_PROTO_TCP) ||
+ (ena_rx_ctx->l4_proto == ENA_ETH_IO_L4_PROTO_UDP)))
unnecessary parentheses, as explained in the previous example.
CHECK: Unnecessary parentheses around 'ena_rx_ctx->l4_proto ==
ENA_ETH_IO_L4_PROTO_UDP'
#1596: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:1596:
+ if (likely((ena_rx_ctx->l4_proto == ENA_ETH_IO_L4_PROTO_TCP) ||
+ (ena_rx_ctx->l4_proto == ENA_ETH_IO_L4_PROTO_UDP)))
unnecessary parentheses, as explained in the previous example.
CHECK: Please use a blank line after function/struct/union/enum declarations
#1636: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:1636:
+}
+/* ena_clean_rx_irq - Cleanup RX irq
message not present in docs for checkpatch.
we should use a blank line after declaring a function/struct/enum.
adding a blank line after the closing brace should remove the message.
CHECK: Alignment should match open parenthesis
#1820: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:1820:
+static void ena_unmask_interrupt(struct ena_ring *tx_ring,
+ struct ena_ring *rx_ring)
the 2nd line not aligned properly with the opening parenthesis of the
first.
explained in a previous example.
CHECK: Alignment should match open parenthesis
#1852: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:1852:
+static void ena_update_ring_numa_node(struct ena_ring *tx_ring,
+ struct ena_ring *rx_ring)
the 2nd line not aligned properly with the opening parenthesis of the
first.
explained in a previous example.
WARNING: Unnecessary ftrace-like logging - prefer using ftrace
#2621: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:2621:
+ netif_dbg(adapter, ifup, adapter->netdev, "%s\n", __func__);
WARNING: Unnecessary ftrace-like logging - prefer using ftrace
#2683: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:2683:
+ netif_info(adapter, ifdown, adapter->netdev, "%s\n", __func__);
WARNING: Unnecessary ftrace-like logging - prefer using ftrace
#2771: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:2771:
+ netif_dbg(adapter, ifdown, netdev, "%s\n", __func__);
no explanation for this message was available in the documentation for
checkpatch.
To my understanding, ftrace is a function that is used in development of
linux kernel to understand and analyse any function that takes place out
of user-space.
so here, i believe that netif_dbg, netif_info, and netif_dbg are
functions that
carry out a task that can also be done by ftrace, which could be even
more efficient.
so usage of ftrace is preferred so that we get better information of that
function which is analysed.
CHECK: Unnecessary parentheses around 'skb->ip_summed == CHECKSUM_PARTIAL'
#2855: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:2855:
+ if ((skb->ip_summed == CHECKSUM_PARTIAL) || mss) {
unnecessary parentheses, as explained in the previous example.
CHECK: Unnecessary parentheses around 'num_frags == tx_ring->sgl_size'
#2912: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:2912:
+ if ((num_frags == tx_ring->sgl_size) &&
+ (header_len < tx_ring->tx_max_header_size))
unnecessary parentheses, as explained in the previous example.
CHECK: Unnecessary parentheses around 'header_len <
tx_ring->tx_max_header_size'
#2912: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:2912:
+ if ((num_frags == tx_ring->sgl_size) &&
+ (header_len < tx_ring->tx_max_header_size))
unnecessary parentheses, as explained in the previous example.
WARNING: LINUX_VERSION_CODE should be avoided, code should be for the
version to which it is merged
#3168: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:3168:
+ host_info->kernel_ver = LINUX_VERSION_CODE;
an exact number of the kernel version to which it will be merged
should be assigned to host_info->kernel_ver rather than another variable.
WARNING: Prefer strscpy over strlcpy - see:
https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/ (local)
#3169: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:3169:
+ strlcpy(host_info->kernel_ver_str, utsname()->version,
strlcpy() function limits the destination size, and is depreciated.
using strscpy() should remove the warning.
CHECK: line length of 101 exceeds 100 columns
#3365: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:3365:
+ struct ena_llq_configurations *llq_default_configurations)
line length exceeds 100 columns. message will be removed if it is split
to multiple lines.
explained in the docs.
CHECK: Alignment should match open parenthesis
#3373: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:3373:
+ dev_warn(&pdev->dev,
+ "LLQ is not supported Fallback to host mode policy.\n");
the 2nd line not aligned properly with the opening parenthesis of the
first.
explained in a previous example.
WARNING: memory barrier without comment
#3698: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:3698:
+ smp_mb__before_atomic();
WARNING: memory barrier without comment
#3737: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:3737:
+ smp_mb__before_atomic();
no documentation is given for this message in checkpatch.
to my understanding, this is caused because there is no
proper comment to this function call giving the context on why it is
called in that line.
adding a comment that gives proper context should remove the message.
CHECK: line length of 119 exceeds 100 columns
#3747: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:3747:
+ time_since_last_napi = jiffies_to_usecs(jiffies -
tx_ring->tx_stats.last_napi_jiffies);
CHECK: line length of 105 exceeds 100 columns
#3748: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:3748:
+ missing_tx_comp_to = jiffies_to_msecs(adapter->missing_tx_completion_to);
CHECK: line length of 104 exceeds 100 columns
#3751: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:3751:
+ tx_ring->qid, i, time_since_last_napi, missing_tx_comp_to);
line length exceeds 100 columns. message will be removed if it is split
to multiple lines.
CHECK: Please don't use multiple blank lines
#4139: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:4139:
+
+
multiple blank lines caused the message. removing them should do it.
WARNING: Unnecessary ftrace-like logging - prefer using ftrace
#4228: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:4228:
+ dev_dbg(&pdev->dev, "%s\n", __func__);
using ftrace is encouraged instead of the above fn. explained in an
example before.
CHECK: Unnecessary parentheses around 'adapter->msix_vecs >= 1'
#4464: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:4464:
+ if ((adapter->msix_vecs >= 1) && (netdev->rx_cpu_rmap)) {
CHECK: Unnecessary parentheses around 'netdev->rx_cpu_rmap'
#4464: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:4464:
+ if ((adapter->msix_vecs >= 1) && (netdev->rx_cpu_rmap)) {
discussed in a previous example.
WARNING: Unnecessary ftrace-like logging - prefer using ftrace
#4612: FILE: drivers/net/ethernet/amazon/ena/ena_netdev.c:4612:
+ netif_dbg(adapter, ifup, adapter->netdev, "%s\n", __func__);
discussed in a previous example.
total: 0 errors, 10 warnings, 23 checks, 4689 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or
--fix-inplace.
drivers/net/ethernet/amazon/ena/ena_netdev.c has style problems, please
review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS
--------------------------------------------------------------------------------------------------------------
I hope I have answered all the questions asked as per the first task. If
there is any clarification or further explanation required,
please do let me know so that i can expand further on this answer. Thank
you for this opportunity. I eagerly await my second task.
sincerely,
Rahul.