Re: [PATCH] rtnetlink.7: Document struct ifa_cacheinfo

Alejandro Colomar
2024-11-11T09:34:49Z

[-- Attachment #1: Type: text/plain, Size: 3234 bytes --]

Bon dia Alex, :)

On Sun, Nov 10, 2024 at 11:17:40PM GMT, Alex Henrie wrote:
> On Tue, Nov 5, 2024 at 4:33 AM Alejandro Colomar <alx@kernel.org> wrote:
>
> > On Mon, Nov 04, 2024 at 09:53:38PM GMT, Kuniyuki Iwashima wrote:
> > > From: Alex Henrie <alexhenrie24@gmail.com>
> > > Date: Mon, 4 Nov 2024 21:14:20 -0700
> > > > struct ifa_cacheinfo contains the address's creation time, update time,
> > > > preferred lifetime, and valid lifetime. See
> >
> > We use two spaces after period (the correct amount). :)
> > (I'm thinking we probably want to document something about it in
> > man-pages(7). Branden, do you want to send a patch about it? I want
> > to include the references you showed to me, and you probably remember
> > better those links.)
> >
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/if_addr.h?h=v6.11#n60
> >
> > Please use this format for links:
> >
> > Link: <http://example.com>
>
> Since the second sentence will be eliminated in favor of a Link line,
> the first sentence will no longer have any spaces after its period.
>
> > Which include provides the structure?

The manual page should document it. Readers should know which header
they need to include to use a structure, no?

>
> linux/if_addr.h, which is the file I linked to in the commit message,
> and the same file that contains struct ifaddrmsg which is documented a
> few paragraphs earlier in the same section of the man page.
>
> > > > +struct ifa_cacheinfo {
> > > > + __u32 ifa_prefered; /* Preferred lifetime in seconds, -1 = forever */
> > > > + __u32 ifa_valid; /* Valid lifetime in seconds, -1 = forever */
> > >
> > > -1 should be rather 0xFFFFFFFF (INFINITY_LIFE_TIME) as it's unsigned.
> >
> > I prefer UINT32_MAX over 0xF...F, which might be unclear how many Fs it
> > has.
>
> INFINITY_LIFE_TIME is not defined in any public header, so let's not
> mention it. I agree that it's hard to see at a glance how many F's are
> in 0xFFFFFFFF. I would suggest ~0u, which is short and sweet, but
> UINT32_MAX is a little better because ~0u isn't 32 bits on all C
> compilers that have ever existed.
>
> > > Also, it would be nice to mention that ifa_prefered must be less than
> > > or equal to ifa_valid (ifa_prefered <= ifa_valid) and 0 is invalid for
> > > ifa_valid.
> > >
> > > 0 <= ifa_prefered <= ifa_valid
> > > 0 < ifa_valid <= 0xFFFFFFFF
>
> I'll add a paragraph to explain those relationships.
>
> > It might also be interesting to add a separate manual page for the type,
> > and reference it here. Otherwise, the page starts getting fatty.
>
> Perhaps. In my opinion, there's not enough material here to be worthy
> of its own page.

If you see the pages in section 2type or 3type, they're pretty small.
Types don't have that much to tell. But they're nevertheless useful.
When you just want to see which header you need for a given type, just
run `make type`, and you'll immediately see it. It also has a few lines
of description.

But up to you.

> Thanks for the feedback,

Have a lovely day!
Alex

>
> -Alex
>

--
<https://www.alejandro-colomar.es/>