Re: [PATCH v2] docs: document linked lists

[Randy Dunlap <rdunlap@infradead.org>]

Hi,

Just a few comments while I am still reviewing:


On 7/3/25 7:10 AM, Jonathan Corbet wrote:
> Thanks for doing this!
> 
> I have a few comments, most of which are just nits.  I think we should
> be able to get this in for 6.17.
> 
> Nicolas Frattaroli <nicolas.frattaroli@collabora.com> writes:
> 


>> diff --git a/Documentation/core-api/list.rst b/Documentation/core-api/list.rst
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..b0586056abb04d2bcc4518f7238ff9a94d3dd774
>> --- /dev/null
>> +++ b/Documentation/core-api/list.rst
>> @@ -0,0 +1,847 @@
>> +.. SPDX-License-Identifier: GPL-2.0+
>> +
>> +=====================
>> +Linked Lists in Linux
>> +=====================
>> +
>> +:Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

> 
> I do wonder if you should start by showing the list_head structure
> itself?  It is simple enough and not a secret that needs to be kept.

+1

>> +Declaring and initializing a list
>> +---------------------------------
>> +
>> +A doubly-linked list can then be declared as just another ``struct list_head``,
>> +and initialised with the LIST_HEAD_INIT() macro during initial assignment, or
>> +with the INIT_LIST_HEAD() function later:
>> +
>> +.. code-block:: c
>> +
>> +  struct clown_car {
>> +          int tyre_pressure[4];
>> +          struct list_head clowns;        /* Looks like a node! */
>> +  };
>> +
>> +  /* ... Somewhere later in our driver ... */
>> +
>> +  static int circus_init(struct circus_priv *circus)
>> +  {
>> +          struct clown_car other_car = {
>> +                .tyre_pressure = {10, 12, 11, 9},
>> +                .clowns = LIST_HEAD_INIT(other_car.clowns)
>> +          };
>> +
>> +          circus->car.clowns = INIT_LIST_HEAD(&circus->car.clowns);

linked_lists.c: In function ‘circus_init’:
linked_lists.c:35:30: error: invalid use of void expression
   35 |           circus->car.clowns = INIT_LIST_HEAD(&circus->car.clowns);

due to
static inline void INIT_LIST_HEAD(struct list_head *list);

Should it just be:
		INIT_LIST_HEAD(&circus->car.clowns);
?

>> +
>> +          return 0;
>> +  }
>> +
>> +A further point of confusion to some may be that the list itself doesn't really
>> +have its own type. The concept of the entire linked list and a
>> +``struct list_head`` member that points to other entries in the list are one and
>> +the same.

> [...]
> 
> 
>> +Further reading
>> +---------------
>> +
>> +* `How does the kernel implements Linked Lists? - KernelNewbies <https://kernelnewbies.org/FAQ/LinkedLists>`_
> 
> I do still think you should move the kerneldoc for lists over from
> kernel-api.rst; just tack it onto the end here.

Ack.

-- 
~Randy