Researching for a discussion at https://gerrit.osmocom.org/c/libosmocore/+/12384 made me realize that our use of doxygen keywords is currently confused.
TLDR:
- Links to structs, members and functions are implicit in doxygen. - Don't use '\ref', except to link to sections or files. - Don't use '\a', it is merely cosmetic and breaks reading flow for some.
Details:
(*) We are often using '\ref' wrongly, and also '\a' is merely cosmetic.
Example: osmo_crc16(), see param 'len' intending to reference the 'buffer' param:
/*! Compute 16bit CCITT polynome 0x8408 (x^0 + x^5 + x^12) over given buffer. * \param crc[in] previous CRC value * \param buffer[in] data pointer * \param len[in] number of bytes in input \ref buffer * \return updated CRC value */ uint16_t osmo_crc16(uint16_t crc, uint8_t const *buffer, size_t len) ...
It looks perfectly sane, but this is wrong for more than one reason:
- '\ref' is not actually about code elements. It is about referencing pages or code sections. See http://www.doxygen.nl/manual/commands.html#cmdref "Creates a reference to a named section, subsection, page or anchor." Doxygen complains with e.g.: "gsmtap_util.c:192: warning: unable to resolve reference to `data' for \ref command" Luckily '\ref' doesn't break implicit linking, which is probably why the misconception that we need to include them arose in the first place.
- There *AREN'T* any references to function arguments at all!! It is not possible in doxygen to create a formal link to a function argument. You can reference struct members, functions, ... but not arguments.
API code often uses '\a' (at least 320 times in libosmocore), but that does not add a formal link, either. '\a' is only and simply putting a term in italics. http://www.doxygen.nl/manual/commands.html#cmda
There are currently at least 260 uses of '\ref' in libosmocore, of which only a few are correct. An example of correct use is gsmtap_source_init(): http://ftp.osmocom.org/api/latest/libosmocore/core/html/group__gsmtap.html#g... /*! [...] integrated with libosmocore \ref select */ which renders as integrated with libosmocore [Select loop abstraction]
Ironically, the same gsmtap_source_init() also includes examples of incorrect use, ignore those for this argument, please.
(*) Hyperlinks are fully automatic and implicit.
Notice that formal links are detected automatically, without the need for an explicit doxygen marker. For example, look at the API doc for osmo_cgi_name2(): http://ftp.osmocom.org/api/latest/libosmocore/gsm/html/gsm23003_8c.html#af96... It has a link to osmo_cgi_name(), with a doxygen source of /*! Same as osmo_cgi_name(), but uses a different static buffer. [...]
I will hence continue to omit '\a' and '\ref', and will actually ask patch submitters to omit them in code review.
~N
Thanks for the write-up, I've extended the guidelines with "Parameters can not be referenced with \ref, nor with \a" and linked to your post:
https://osmocom.org/projects/cellular-infrastructure/wiki/Guidelines_for_API...
Regards, Oliver
On 1/3/19 1:30 AM, Neels Hofmeyr wrote:
Researching for a discussion at https://gerrit.osmocom.org/c/libosmocore/+/12384 made me realize that our use of doxygen keywords is currently confused.
TLDR:
- Links to structs, members and functions are implicit in doxygen.
- Don't use '\ref', except to link to sections or files.
- Don't use '\a', it is merely cosmetic and breaks reading flow for some.
Details:
(*) We are often using '\ref' wrongly, and also '\a' is merely cosmetic.
Example: osmo_crc16(), see param 'len' intending to reference the 'buffer' param:
/*! Compute 16bit CCITT polynome 0x8408 (x^0 + x^5 + x^12) over given buffer. * \param crc[in] previous CRC value * \param buffer[in] data pointer * \param len[in] number of bytes in input \ref buffer * \return updated CRC value */ uint16_t osmo_crc16(uint16_t crc, uint8_t const *buffer, size_t len) ...
It looks perfectly sane, but this is wrong for more than one reason:
'\ref' is not actually about code elements. It is about referencing pages or code sections. See http://www.doxygen.nl/manual/commands.html#cmdref "Creates a reference to a named section, subsection, page or anchor." Doxygen complains with e.g.: "gsmtap_util.c:192: warning: unable to resolve reference to `data' for \ref command" Luckily '\ref' doesn't break implicit linking, which is probably why the misconception that we need to include them arose in the first place.
There *AREN'T* any references to function arguments at all!! It is not possible in doxygen to create a formal link to a function argument. You can reference struct members, functions, ... but not arguments.
API code often uses '\a' (at least 320 times in libosmocore), but that does not add a formal link, either. '\a' is only and simply putting a term in italics. http://www.doxygen.nl/manual/commands.html#cmda
There are currently at least 260 uses of '\ref' in libosmocore, of which only a few are correct. An example of correct use is gsmtap_source_init(): http://ftp.osmocom.org/api/latest/libosmocore/core/html/group__gsmtap.html#g... /*! [...] integrated with libosmocore \ref select */ which renders as integrated with libosmocore [Select loop abstraction]
Ironically, the same gsmtap_source_init() also includes examples of incorrect use, ignore those for this argument, please.
(*) Hyperlinks are fully automatic and implicit.
Notice that formal links are detected automatically, without the need for an explicit doxygen marker. For example, look at the API doc for osmo_cgi_name2(): http://ftp.osmocom.org/api/latest/libosmocore/gsm/html/gsm23003_8c.html#af96... It has a link to osmo_cgi_name(), with a doxygen source of /*! Same as osmo_cgi_name(), but uses a different static buffer. [...]
I will hence continue to omit '\a' and '\ref', and will actually ask patch submitters to omit them in code review.
~N
-
Neels Hofmeyr -
Oliver Smith