Re: [LTP] doc: Add missing API references to api_c_tests.rst

[linuxtestproject.agent@gmail.com]

Hi Andrea,

Thank you for this documentation series. The kernel-doc conversions and new
reference pages are a welcome addition. However, several commit message claims
do not match what the patches actually implement, and there are technical
issues in tst_fuzzy_sync.h that need to be fixed before merging.

---

[PATCH 1/3] doc: Add missing API references to api_c_tests.rst
04e9aa97059d1301f10954322074c7cb3d9fdd53

--- Commit message ---

> Fix broken kernel-doc reference to renamed tse_numa.h header

This claim is false. The api_c_tests.rst NUMA section still references
`../../include/tst_numa.h` (which does not exist) in both master and HEAD:

  $ cat doc/developers/api_c_tests.rst | grep -n numa
  73:NUMA
  75:.. kernel-doc:: ../../include/tst_numa.h

The actual file is include/tse_numa.h. The reference was not corrected by
this patch.

> Convert plain comments to kernel-doc format in tst_device.h,
> tst_hugepage.h, tst_clone.h, and tst_taint.h

tst_fuzzy_sync.h also receives kernel-doc conversions (old @param/@return
Doxygen style → new function() - description / @param: style) but is not
mentioned here.

--- include/tst_fuzzy_sync.h ---

The patch adds new-style kernel-doc parameters to several functions but fails
to remove the old Doxygen-style @param / @return lines. This leaves duplicate,
conflicting documentation in the following functions:

tst_exp_moving_avg():

  /**
   * tst_exp_moving_avg() - Exponential moving average.
   * @alpha: Smoothing factor.          ← new style
   * @sample: New sample value.
   * @prev_avg: Previous average.
   *
   * Return: New average.
   *
   * @param alpha The preference for recent samples over old ones.  ← stale
   * @param sample The current sample                               ← stale
   * @param prev_avg The average of the all the previous samples    ← stale
   *
   * @return The average including the current sample.              ← stale
   */

@param alpha and @param sample were removed by the patch, but @param prev_avg
and @return were not. They should be removed.

tst_fzsync_pair_reset():

  /**
   * tst_fzsync_pair_reset() - Reset or initialise fzsync.
   * @pair: Fuzzy sync pair.         ← new style
   * @run_b: Thread B function pointer.
   *
   * @relates tst_fzsync_pair
   * @param pair The state structure initialised with TST_FZSYNC_PAIR_INIT.  ← stale
   * @param run_b The function defining thread B or NULL.                    ← stale
   */

@param pair and @param run_b were not removed. They should be removed.

tst_fzsync_pair_wait():

  /**
   * tst_fzsync_pair_wait() - Wait for the other thread.
   * @our_cntr: Our atomic counter.        ← new style
   * @other_cntr: Other thread's atomic counter.
   * @spins: Spin counter.
   * @exit: Exit flag.
   * @yield_in_wait: Whether to yield while waiting.
   *
   * @relates tst_fzsync_pair
   * @param our_cntr The counter for the thread we are on    ← stale
   * @param other_cntr ...                                   ← stale
   * @param spins ...                                        ← stale
   * @param exit ...                                         ← stale
   * ...
   * @return A non-zero value if the thread should continue  ← stale
   */

All four @param lines and the @return line were not removed.

tst_fzsync_run_a():

  /**
   * tst_fzsync_run_a() - Decide whether to continue running thread A.
   * @pair: Fuzzy sync pair.
   *
   * Return: Non-zero to continue, 0 to stop.   ← new style
   *
   * ...
   * @return True to continue and false to break.  ← stale, not removed
   */

tst_fzsync_pair_add_bias():

  /**
   * tst_fzsync_pair_add_bias() - Add some amount to the delay bias.
   * @pair: Fuzzy sync pair.
   * @change: Amount to add to bias.    ← new style
   *
   * @relates tst_fzsync_pair
   * @param change The amount to add, can be negative  ← stale, not removed
   */

All stale @param / @return / @relates lines left after the conversion should
be removed.

---

[PATCH 2/3] doc: Complete struct tst_test table and shell API docs
f1c9189e4415389807671809f27d2b922e36e99b

--- Commit message ---

The body describes four fixes to writing_tests.rst that are NOT present in
the patch:

1. ".max_runtime -> .runtime (renamed in a6a369c5eeb4)"

   Not done. writing_tests.rst still has:

     * - .max_runtime
       - TST_TIMEOUT ...

   The actual struct field is `int runtime` (include/tst_test.h:599). The
   table entry should be `.runtime`, not `.max_runtime`.

2. ".tst_hugepage -> .hugepages (actual field name)"

   Not done. writing_tests.rst still has:

     * - .tst_hugepage
       - not applicable

   The actual struct field is `struct tst_hugepage hugepages`
   (include/tst_test.h:586). The table entry should be `.hugepages`.

3. ".dev_extra_opts, .dev_fs_opts, .dev_fs_type are shell-only variables,
   not C struct fields; move them to the 'not applicable' section"

   Not done. These three rows remain unchanged in the table, still listed
   as if they were C struct fields. None of these names appear in
   include/tst_test.h.

4. "Add 10 missing struct tst_test fields"

   Only 8 fields are added: .filesystems, .forks_child, .min_runtime,
   .needs_abi_bits, .needs_cgroup_nsdelegate, .needs_cgroup_ver,
   .needs_hugetlbfs, .runs_script. The commit body also lists exactly 8
   names. The subject and body should say 8, not 10.

The commit message must be corrected to describe only what the patch
actually does, and the four unimplemented fixes above must either be
implemented or dropped from the description.

---

[PATCH 3/3] doc: Add SAFE_* macros reference page
c9f08715e5e82b5a07d355c6c26313e4608cb3e4

--- doc/developers/api_safe_macros.rst ---

> +.. Copyright (c) Linux Test Project, 2025

This file is being created in 2026; the year should be 2026 (or 2025-2026
if the content dates to 2025, but for a new file 2026 is appropriate).
Compare api_c_tests.rst which uses "2024-2025".

> +- ``SAFE_MMAP(addr, length, prot, flags, fd, offset)``

This line appears in the "Core macros (tst_safe_macros.h)" Memory management
section. However, SAFE_MMAP is defined only in include/tst_safe_macros_inline.h,
not in tst_safe_macros.h. It is also correctly listed in the "Inline macros
(tst_safe_macros_inline.h)" section. Remove the duplicate entry from the
core macros section; it belongs only under "Inline macros".

Because of this duplicate, the "all 217 SAFE_* wrapper macros" claim in the
subject overstates by one; there are 216 unique macros. The count and subject
line should be corrected after removing the duplicate.

---

Verdict: Needs revision

LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp