Project

General

Profile

Actions

Documentation #4743

open

Improve Suricata code documentation (C files)

Added by Juliana Fajardini Reichow over 2 years ago. Updated 5 months ago.

Status:
New
Priority:
Normal
Target version:
Affected Versions:
Effort:
Difficulty:
Label:
Beginner, C, Good First Issue, Outreachy

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 :)


Subtasks 1 (0 open1 closed)

Documentation #6383: misc: improve code documentationClosedLiza OparActions
Actions #1

Updated by Juliana Fajardini Reichow over 2 years ago

  • Description updated (diff)
Actions #2

Updated by Sophia Abubakar over 2 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 ?

Actions #3

Updated by Juliana Fajardini Reichow over 2 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! :)

Actions #4

Updated by Juliana Fajardini Reichow over 2 years ago

  • Assignee set to Community Ticket
Actions #5

Updated by Juliana Fajardini Reichow over 1 year ago

Unclaimed first group for lack of progress on the task for the last 6 months.

Actions #6

Updated by Juliana Fajardini Reichow over 1 year ago

  • Target version set to TBD
Actions #7

Updated by Liza Opar 6 months ago

Hi i'd like to work on this issue.

Actions #8

Updated by Juliana Fajardini Reichow 6 months ago

  • Subtask #6383 added
Actions #9

Updated by Liza Opar 6 months ago

  • Assignee changed from Community Ticket to Liza Opar
Actions #10

Updated by Juliana Fajardini Reichow 6 months 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 .

Actions #11

Updated by Juliana Fajardini Reichow 6 months ago

  • Assignee changed from Liza Opar to Community Ticket
Actions #12

Updated by Liza Opar 6 months ago

alright

Actions #13

Updated by Hadiqa Alamdar Bukhari 5 months ago

Hi, I'd like to claim group 4 for this ticket.

Actions #14

Updated by Juliana Fajardini Reichow 5 months 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?

Actions #15

Updated by Hadiqa Alamdar Bukhari 5 months 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.

Actions

Also available in: Atom PDF