Documentation #4743
openImprove Suricata code documentation (C files)
Description
Suricata project shares its C codebase documentation with Doxygen: https://doxygen.openinfosecfoundation.org/
But some Suricata c files (.c and .h) are still missing more informative documentation that makes use of Doxygen
annotation.
Why is that important? Besides improving the documentation for other developers who seek to contribute to Suricata,
if Doxygen provides a feature such as Rust's cargo ability to check for code and documentation synchronization, then,
if the code is changed and the documentation isn't updated accordingly, we'll get a warning that will allow us to keep
the documentation up to date with the codebase changes.
For this issue, it suffices to add something like the following (snippet from: detect-asn1.c):
/**
* \file detect-asn1.c
*
* Implements "asn1" keyword
*/
Some of the files already have the \file tag but are missing a brief description. Some are missing both.
If a file doesn't have a description, it will likely be necessary to study the source code a bit before knowing what to write.
As an extra bonus task, several functions need documentation on parameters and return values. Because this asks for a deeper
understanding of the Suricata codebase, it is ok to leave this out for the first PR. Consider as example of a well-documented function
declaration, from app-layer-detect-proto.h:
/**
* \brief Returns the app layer protocol given a buffer.
*
* \param tctx Pointer to the app layer protocol detection thread context.
* \param f Pointer to the flow.
* \param buf The buffer to be inspected.
* \param buflen The length of the above buffer.
* \param ipproto The ip protocol.
* \param flags The direction bitfield - STREAM_TOSERVER/STREAM_TOCLIENT.
* \param[out] reverse_flow true if flow is detected to be reversed
*
* \retval The app layer protocol.
*/
AppProto AppLayerProtoDetectGetProto(AppLayerProtoDetectThreadCtx *tctx, Flow *f,
const uint8_t *buf, uint32_t buflen, uint8_t ipproto, uint8_t flags, bool *reverse_flow);
To work on this issue, you should claim a group of files to which you shall add the proper Doxygen tags.
Check the file groups at: https://docs.google.com/spreadsheets/d/1Pwjq3Q2vykZSGXHnwRh76ZTJWW6XUYnxCneW7iuTzyg/edit?usp=sharing
Claiming steps:
- If you haven't done so yet, register a Redmine account for the OISF project: https://redmine.openinfosecfoundation.org/account/register;
- Visit the file list and choose a group of files which hasn't been claimed yet;
- Ask if you can claim a group in this ticket (mention the group number). Bear in mind that we may take a few hours to respond;
Once we have agreed on the claim, we'll answer with a comment in this ticket and update the spreadsheet with your Redmine username,
to show that someone is already working on it;
- Share your progress on GitHub pull requests;
- Enjoy the ride! :)
Please note that:
- This ticket is an umbrella ticket (it's not for claiming!);
- This type of issue is meant to offer a good starting point for those who want to begin contributing to Suricata
but are not familiar with Open Source projects and/or our contributing guidelines. Folks may claim only one group of files to work on.
Finished your work with this task? You are welcome to claim a more complex issue. (Not sure what those could be? Try filtering tickets with "beginners" label);
- Please, do not add the group reference number to git commits, commit messages, nor any of such. Those are for internal control, only =]
If you haven't done that yet, please check our Contributing guide for more detailed instructions on our workflow :)
Updated by Juliana Fajardini Reichow about 3 years ago
- Description updated (diff)
Updated by Sophia Abubakar almost 3 years ago
Juliana Fajardini Reichow wrote:
- Ask if you can claim a group in this ticket (mention the group number). Bear in mind that we may take a few hours to respond;
Can I claim group 1 ?
Updated by Juliana Fajardini Reichow almost 3 years ago
Hi Sophia, yes, I'll mark that one as claimed by you. Please comment here in this issue sharing the PR link, when you're ready. Thank you! :)
Updated by Juliana Fajardini Reichow almost 3 years ago
- Assignee set to Community Ticket
Updated by Juliana Fajardini Reichow about 2 years ago
Unclaimed first group for lack of progress on the task for the last 6 months.
Updated by Juliana Fajardini Reichow almost 2 years ago
- Target version set to TBD
Updated by Liza Opar about 1 year ago
- Assignee changed from Community Ticket to Liza Opar
Updated by Juliana Fajardini Reichow about 1 year ago
Liza Opar wrote in #note-7:
Hi i'd like to work on this issue.
Hey @Liza Opar, as this is the parent issue, this one shouldn't be claimed by a specific person. That's why I created a subtask for you. I'll unclaim the ticket, since you'll be working on #6383 .
Updated by Juliana Fajardini Reichow about 1 year ago
- Assignee changed from Liza Opar to Community Ticket
Updated by Hadiqa Alamdar Bukhari about 1 year ago
Hi, I'd like to claim group 4 for this ticket.
Updated by Juliana Fajardini Reichow about 1 year ago
Hadiqa Alamdar Bukhari wrote in #note-13:
Hi, I'd like to claim group 4 for this ticket.
Hi Hadiqa, would you rather work with this task, or this one - https://redmine.openinfosecfoundation.org/issues/6356#change-30396?
I'd suggest you picked only one, and tried another once you've at least submitted one PR for it. Feel free to choose which of the two you'd like to work on, and please let us know, ok?
Updated by Hadiqa Alamdar Bukhari about 1 year ago
Juliana Fajardini Reichow wrote in #note-14:
Hi Hadiqa, would you rather work with this task, or this one - https://redmine.openinfosecfoundation.org/issues/6356#change-30396?
I'd suggest you picked only one, and tried another once you've at least submitted one PR for it. Feel free to choose which of the two you'd like to work on, and please let us know, ok?
Okay, I'll work on https://redmine.openinfosecfoundation.org/issues/6356#change-30396 first and then I'll shift to this one.