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

[linuxtestproject.agent@gmail.com]

Hi Andrea,

thank you for the documentation series. A few issues below.

---

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

Claim: "listing all 245 SAFE_* wrapper macros"
Verified: 252 unique SAFE_* macro names in include/; the 7 not listed
are header guards (SAFE_MACROS_FN_H__, SAFE_NET_FN_H__,
SAFE_STDIO_FN_H__, SAFE_RT_SIGNAL_H__) and internal helpers
(SAFE_FILE_OPS, SAFE_FILE_OPS_FN, SAFE_IOCTL_).  Count 245 is correct.

No other issues found in this commit.

---

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

Claim: "Add 8 missing struct tst_test fields"
Verified: .filesystems, .forks_child, .min_runtime, .needs_abi_bits,
.needs_cgroup_nsdelegate, .needs_cgroup_ver, .needs_hugetlbfs,
.runs_script — all present in include/tst_test.h.  Count 8 is correct.

Alphabetical fixup (.mnt_flags ↔ .min_swap_avail): verified correct.

Shell API variables: spot-checked TST_NEEDS_KCONFIGS_IFS, TST_FS_TYPE
(default ext2), TST_TEST_DATA_IFS (default space), checkpoint functions
and retry helpers — all accurate.

No issues found in this commit.

---

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

Claim: "10 headers" added.
Verified in the api_c_tests.rst diff:
  tse_uinput.h, tst_clone.h, tst_device.h, tst_fs.h,
  tst_fuzzy_sync.h, tst_hugepage.h, tst_kconfig.h, tst_memutils.h,
  tst_safe_file_ops.h, tst_taint.h — count 10 is correct.

Claim: removing @param, @return, @relates, @sa Doxygen directives.
Verified: @param in tst_kconfig.h, @return in tst_device.h,
@sa and @relates in tst_fuzzy_sync.h — all removed. Correct.

BUG 1 — include/tst_device.h: missing blank line

After the patch, tst_mount_has_opt()'s declaration is immediately
followed by the kernel-doc comment for tst_is_mounted() with no
blank line between them.  Every other consecutive function pair in
the file (tst_is_mounted / tst_is_mounted_ro, tst_is_mounted_ro /
tst_is_mounted_rw, etc.) has a blank line separating the declaration
from the next /** block.  Please add the missing blank line:

> int tst_mount_has_opt(const char *path, const char *opt);
> /**
>  * tst_is_mounted() - Check if a path is a mount point.

Should be:

  int tst_mount_has_opt(const char *path, const char *opt);

  /**
   * tst_is_mounted() - Check if a path is a mount point.

BUG 2 — include/tst_kconfig.h: TST_KCONFIG_INIT() drops string.h note

The original comment read:
  "Initialization helper macro for struct tst_kconfig_var.
   Requires <string.h>"

The new kernel-doc comment omits the <string.h> requirement:
  "TST_KCONFIG_INIT() - Initialization helper macro for struct
   tst_kconfig_var."

The macro expands to { .id_len = strlen(confname) }, which requires
strlen().  tst_kconfig.h includes only <stdbool.h> and <stddef.h>,
not <string.h>.  The original "Requires <string.h>" note was there for
a reason.  Please preserve it, e.g.:

  /**
   * TST_KCONFIG_INIT() - Initialization helper macro for struct
   *     tst_kconfig_var. Requires <string.h> for strlen().
   * @confname: Config variable name string.
   */

MINOR — include/tst_device.h: tst_device global uses /* not /**

The tst_device extern pointer:

  /*
   * tst_device - Pointer to the test block device.
   * ...
   */
  extern struct tst_device *tst_device;

uses a plain /* comment, so the kernel-doc extractor will not emit an
entry for it even though api_c_tests.rst now includes
`.. kernel-doc:: ../../include/tst_device.h`.  The struct tst_device
doc block above already carries the "Automatically initialized if
tst_test.needs_device is set." note, so this may be intentional.
If the intent is to have the variable appear in the API docs, change
/* to /**.

---

Verdict: Needs revision (BUG 1 and BUG 2 above).

LTP AI Reviewer

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