Skip to content

feat(doc): Adding Doxygen documentation to core files#307

Open
0utd4ted wants to merge 24 commits into
bao-project:mainfrom
0utd4ted:wip/doxygen
Open

feat(doc): Adding Doxygen documentation to core files#307
0utd4ted wants to merge 24 commits into
bao-project:mainfrom
0utd4ted:wip/doxygen

Conversation

@0utd4ted

Copy link
Copy Markdown
Contributor

This is still a very early work in progress and the proposed documentation is still under an initial review for QA.
Get doxygen and run it in the repository to produce an HTML output in "build/doxygen/html".

@0utd4ted 0utd4ted changed the title feat(doc): Adding Doxygen documentation to Bao Hypervisor Core files feat(doc): Adding Doxygen documentation to core files Jan 13, 2026
@danielRep

Copy link
Copy Markdown
Member

Thanks for the contribution @AAGandomi ! I'll try to take a look in the following days

@josecm josecm self-assigned this Jan 16, 2026
@josecm

josecm commented Jan 16, 2026

Copy link
Copy Markdown
Member

Hey Aras, thanks for this (I expect, first!) PR.

I'll also be reviewing it in the coming weeks!

@0utd4ted

0utd4ted commented Jan 21, 2026

Copy link
Copy Markdown
Contributor Author

Almost done with mem.c and vm.c. I will push the second and last set of commits during this coming weekend (by 01/02/2025, things came up).
They will also take care of some issues with sign-off strings in commit messages (expect a force push to update this PR) and some oversights from my part (e.g. cache.c).

@0utd4ted 0utd4ted requested a review from joaopeixoto13 as a code owner March 16, 2026 01:14
@0utd4ted 0utd4ted force-pushed the wip/doxygen branch 2 times, most recently from 7b4ed62 to 60f2cdb Compare March 17, 2026 01:09
@0utd4ted

Copy link
Copy Markdown
Contributor Author

Hello @josecm and @danielRep ,
I will fix the current issues with the CI, don't worry. I think we can start to review it.

I made myself familiar to https://bao-project.readthedocs.io/en/latest/development/code_documentation.html but I have prioritize content over form. My main focus for this contribution was to capture the objective of the functions and their coupling with the rest of the software. This is meant to be a starting point.

The guidelines I have tried my best to stick to:

  1. The @brief summarize in a sentence the goal of the function (its main output).
  2. The "long description" had 2 objectives: describe in more details certain design choices or expand on a very generic brief description ("initialize", I'm looking at you) and highlight non-functional characteristics: logical sequences mostly, since there are no timing constraint (at this time ).
  3. Never use the third person when describing the function. Use imperative form for concise descriptions.
  4. Document all the functions in .c files. macros/inline functions defined in header files are left undocumented.

I have intentionally deviated from Bao's guidelines' form, which may be reverted or fixed in future iterations:

  1. @pre, @post and @invariant where not used - too much effort to do correctly for a draft.
  2. @note was used very liberally, to remind the reader of the presence of alternative outputs (e.g., console messages). Or any behavior that was obvious form the brief description.
  3. @see has been used to list (as a best effort) all global variables, functions (static, local and external) and structs. It was done mostly as an effort to collect functions' any and all dependencies.
  4. @param is assumed as input if not specified otherwise.

I can see the following improvements to be made (on top of a regular review) before merging:

  • Custom doxygen special commands:
    • for placeholder functions (e.g. @notimplemented @archspecific @platformspecific.
    • for alternative intended (and unintended) outputs, as such console warnings, console errors, and exceptions (i.e. jumps to exception vector).
    • for dead ends (e.g. @noreturn)
  • @post when global variables are set.
  • @pre/@invariant when global variables are read.

Comment thread src/core/cache.c Outdated
Comment thread src/core/cache.c
Comment thread src/core/cache.c
Comment thread src/core/config.c Outdated
Comment thread src/core/console.c Outdated
Comment thread src/core/console.c Outdated
Comment thread src/core/cpu.c Outdated
Comment thread src/core/interrupts.c Outdated
Comment thread src/core/interrupts.c Outdated
Comment thread src/core/interrupts.c Outdated
Comment thread src/core/interrupts.c Outdated
Comment thread src/core/interrupts.c
Comment thread src/core/interrupts.c Outdated
Comment thread src/core/interrupts.c Outdated
Comment thread src/core/ipc.c Outdated
0utd4ted and others added 18 commits April 13, 2026 22:16
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
All the fixes resulting from everyone's review will be squashed here.

Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Co-authored-by: Daniel Oliveira <drawnpoetry@gmail.com>
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Co-authored-by: Daniel Oliveira <drawnpoetry@gmail.com>
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
@0utd4ted

Copy link
Copy Markdown
Contributor Author

Hi @AAGandomi ,

My overall feeling of the review for your PR is that you have done an excellent job. I don't have major changes for now, only some typos, grammar in-corrections, and style/format inconsistencies and improvements (in my view).

I believe @josecm will help to identify if I missed something.

* No @return tags for some void functions. No hard feelings for that, but inconsistent with some other files.

* I feel that the long description should be split as in the `console_init` function. Seems more readable.

* Should we change this:
 * @return enum irq_res FORWARD_TO_VM if sent to VM
 * @return enum irq_res HANDLED_BY_HYP if handled by hypervisor
 * @return ERROR if the interrupt ID is not assigned.

For something like this as we do in the multi-line @see tag?

 * @return enum irq_res FORWARD_TO_VM if sent to VM
 *         enum irq_res HANDLED_BY_HYP if handled by hypervisor
 *         ERROR if the interrupt ID is not assigned.

Hello @danielRep ,

  • According to the observed doxygen behaviour, void functions shall not have return tags. I have removed those that do.
  • I agree that the console_init style may be more readable. I will spin a quick script to align all the docs to it.
  • The see and return tags do not work the same way: the see tag is just a list, while each return tag can make the generated documentation easier to read. IMHO the use of multiple return tags should be up to whom write the doc (and the reviewers).

Just for reference, this is generated when using 3 different return tags:
Screenshot_20260416_125522

While this is the doxygen output when only one is used (as you suggested):
Screenshot_20260416_125613

The indented format used for see tags was used to keep all the elements in the same tag. I have removed the use of multiple tags, but I don't think it is a policy - multiple see tags should not affect the final product.

For context: I have introduced those see tags for collecting (manually) data in order to evaluate connascence scores. see tags may very well be removed without affecting the documentation.

@josecm @danielRep IMHO the only changes left for this PR are for linting (and CI issues). I will follow up with a new PR on CI/workflow to introduce doc linting and doxygen checks.

Corrections have also been done to address inconsistencies
on @return, @see, @file and formatting.
Clarification/fix on mem_broadcast.

Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
This update addresses the Doxygen warnings on deprecated configurations.
The version update itself was performed with doxygen assistance.

On top of it, many improvements to the configuration have been performed
manually (see below).

To improve clarity, the following have been set/altered:

- PROJECT_NAME
- PROJECT_NUMBER
- PROJECT_BRIEF

Furthermore, this commit is part of an effort to document core files'
functions. As such, the following configuration has been altered to
(at least temporarily) prevent warnings and errors from doxygen.

- EXTRACT_LOCAL_CLASSES
- HIDE_UNDOC_MEMBERS
- WARN_IF_UNDOC_ENUM_VAL
- HIDE_UNDOC_MEMBERS
- EXTRACT_LOCAL_CLASSES
- EXCLUDE_PATTERNS

As a clarifying note on EXCLUDE_PATTERNS: files in src/arch and
src/platform, header files and remio.c are brought outside the scope
of Doxygen to avoid errors.

These files either

1. Lack documentation;
2. The documentation is insufficient;
3. The documentation is using custom formatting (e.g. @vm).

In the opinion of the author, `remio.c` requires many QA improvements,
many of which can result in a documentation rewrite.
For this reason, the file has been excluded from doxygen's scope.

The following settings are enabled for strict quality control over
function documentation:

- WARN_IF_INCOMPLETE_DOC
- WARN_NO_PARAMDOC

Finally, the following settings have been enabled to invite the use of
Doxygen in the CI:

- WARN_AS_ERROR
- NUM_PROC_THREADS
- TIMESTAMP

Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
0utd4ted added 2 commits May 10, 2026 02:16
The following actions have been performed:
1. Grammar check and spell check on doxygen documentation.
2. Styling check: empty lines before and after detailed descriptions.
3. Styling check: end tags with a period (to close the sentence or list).

All findings have been reviewed and fixed.

Assisted-by: Qwen 3.5 9b, opencode, harper-cli.
Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
Doxyfile has been modified to produce a warn log (doxywarn.log).
This change should simplify processing any warning or error.
Logs (.log) have been added to .gitignore, to prevent mishaps.

Signed-off-by: Aras Ashraf Gandomi <aras.gandomi@gmail.com>
@danielRep

Copy link
Copy Markdown
Member

First, sorry for the delayed reply @AAGandomi . Very busy in the last weeks, but I'm back to this activity.

Hello @danielRep ,

  • According to the observed doxygen behaviour, void functions shall not have return tags. I have removed those that do.

Agree.

  • I agree that the console_init style may be more readable. I will spin a quick script to align all the docs to it.

Sounds good!

Just for reference, this is generated when using 3 different return tags:

While this is the doxygen output when only one is used (as you suggested):

I see. Let's keep it as you've proposed then.

The indented format used for see tags was used to keep all the elements in the same tag. I have removed the use of multiple tags, but I don't think it is a policy - multiple see tags should not affect the final product.

For context: I have introduced those see tags for collecting (manually) data in order to evaluate connascence scores. see tags may very well be removed without affecting the documentation.

Got it.

@josecm @danielRep IMHO the only changes left for this PR are for linting (and CI issues). I will follow up with a new PR on CI/workflow to introduce doc linting and doxygen checks.

I'm on those PRs too. Let's try to push this in the following weeks as soon as @josecm get's some free time. From my part, I'm just doing a last review and approve if nothing pops up.

@0utd4ted

Copy link
Copy Markdown
Contributor Author

Alright, no problem. I will keep pushing some more stuff here, very small stuff (typos and leftover). As per the merge, I suggest to keep the commits on specific files and then squash any following fix in 62cf5d7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants