Re: [PATCH libnetfilter_log] src: doc: revise doxygen for module "Netlink message helper functions"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 07, 2021 at 01:04:24AM +0200, Pablo Neira Ayuso wrote:
> On Sun, Sep 05, 2021 at 12:33:20PM +1000, Duncan Roe wrote:
> > Adjust style to work better in a man page.
> > Document actual return values.
>
> All good, except one chunk I'm ambivalent.
>
> > Signed-off-by: Duncan Roe <duncan_roe@xxxxxxxxxxxxxxx>
> > ---
> >  src/nlmsg.c | 53 +++++++++++++++++++++++++----------------------------
> >  1 file changed, 25 insertions(+), 28 deletions(-)
> >
> > diff --git a/src/nlmsg.c b/src/nlmsg.c
> > index 3ebb364..399b19a 100644
> > --- a/src/nlmsg.c
> > +++ b/src/nlmsg.c
> > @@ -18,15 +18,15 @@
> >   */
> >
> >  /**
> > - * nflog_nlmsg_put_header - reserve and prepare room for nflog Netlink header
> > - * \param buf memory already allocated to store the Netlink header
> > - * \param type message type one of the enum nfulnl_msg_types
> > - * \param family protocol family to be an object of
> > - * \param qnum queue number to be an object of
> > + * nflog_nlmsg_put_header - convert memory buffer into an nflog Netlink header
>
> this is not just converting as in a cast, I understand "reserve and
> prepare room" might not be so clear to understand.

v2 substitutes populate ... with for convert ... into.
>
> > + * \param buf pointer to memory buffer
> > + * \param type either NFULNL_MSG_PACKET or NFULNL_MSG_CONFIG
>
> I'd keep above a reference to 'enum nfulnl_msg_types'.

v2 appends enum to line. I thought with only 2 enum members one might as well
say what they are so man page user can highlight and paste into code. enum
definition has comments in case member names are not self-explanatory, but man
page user would likely only ever go there once.
>
> > + * \param family protocol family
> > + * \param qnum queue number
>
> I remember you posted a patch to rename qh to gh, from queue handler
> to group handler. You could rename this to 'group number'.

Done in v2. (2 other files involved - hope that's OK with you).
>
> >   *
> > - * This function creates Netlink header in the memory buffer passed
> > - * as parameter that will send to nfnetlink log. This function
> > - * returns a pointer to the Netlink header structure.
> > + * Creates a Netlink header in _buf_ followed by
> > + * a log\-subsystem\-specific extra header.
>
> This function is adding the netlink + nfnetlink headers to the buffer
> as well as setting up the header fields accordingly.

v2 re-words
>
> > + * \return pointer to created Netlink header structure
> >   */
> >  struct nlmsghdr *
> >  nflog_nlmsg_put_header(char *buf, uint8_t type, uint8_t family, uint16_t qnum)

Cheers ... Duncan.



[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux