On 15/11/2024 16:36, Jakub Kicinski wrote:
kernel-doc -Wall warns about missing Return: statement for non-void
functions. We have a number of kdocs in our headers which are missing
the colon, IOW they use
* Return some value
or
* Returns some value
Having the colon makes some sense, it should help kdoc parser avoid
false positives. So add them. This is mostly done with a sed script,
and removing the unnecessary cases (mostly the comments which aren't
kdoc).
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
...
quoted hunk ↗ jump to hunk
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0aae346d919e..ed549a2e02b2 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
...
quoted hunk ↗ jump to hunk
@@ -3548,7 +3548,7 @@ static inline void netdev_tx_sent_queue(struct netdev_queue *dev_queue,
* that they should not test BQL status themselves.
* We do want to change __QUEUE_STATE_STACK_XOFF only for the last
* skb of a batch.
- * Returns true if the doorbell must be used to kick the NIC.
+ * Return true if the doorbell must be used to kick the NIC.
Think the colon went missing here.
quoted hunk ↗ jump to hunk
*/
static inline bool __netdev_tx_sent_queue(struct netdev_queue *dev_queue,
unsigned int bytes,
@@ -3802,7 +3802,7 @@ static inline bool netif_attr_test_mask(unsigned long j,
* @online_mask: bitmask for CPUs/Rx queues that are online
* @nr_bits: number of bits in the bitmask
*
- * Returns true if a CPU/Rx queue is online.
+ * Returns: true if a CPU/Rx queue is online.
*/
static inline bool netif_attr_test_online(unsigned long j,
const unsigned long *online_mask,
@@ -3822,7 +3822,7 @@ static inline bool netif_attr_test_online(unsigned long j,
* @srcp: the cpumask/Rx queue mask pointer
* @nr_bits: number of bits in the bitmask
*
- * Returns >= nr_bits if no further CPUs/Rx queues set.
+ * Returns: >= nr_bits if no further CPUs/Rx queues set.
*/
static inline unsigned int netif_attrmask_next(int n, const unsigned long *srcp,
unsigned int nr_bits)
I agree with Johannes here, it ought to be something like
* Returns: next CPU in mask, or >= nr_bits if no further CPUs/Rx
* queues set.
but understand if you don't want to include a semantic change in
this mechanical reformat patch.
quoted hunk ↗ jump to hunk
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 5c01048860c4..fe0d005cd5d8 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
...> @@ -464,8 +464,8 @@ struct phylink_pcs_ops {quoted hunk ↗ jump to hunk
* mask. Phylink will propagate the changes to the advertising mask. See the
* &struct phylink_mac_ops validate() method.
*
- * Returns -EINVAL if the interface mode/autoneg mode is not supported.
- * Returns non-zero positive if the link state can be supported.
+ * Returns: -EINVAL if the interface mode/autoneg mode is not supported.
+ * Returns: non-zero positive if the link state can be supported.
Does having multiple 'Returns:' sections in kdoc work? I think the
right way to write this is
* Returns:
* * -EINVAL if the interface mode/autoneg mode is not supported.
* * non-zero positive if the link state can be supported.
(Although I'm not sure about the accuracy of this documentation; it
looks like the calling code only treats <0 as error, and several
implementations of the method return 0 in what look like success
cases. So that "non-zero positive" looks sus.)
IDK which part of the patch got me on the CC list but fwiw you can
add my
Reviewed-by: Edward Cree <redacted>
for the whole thing to v2 with the double-Returns fixed.