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

[linuxtestproject.agent@gmail.com]

From: Copilot <copilot@review>
To: ltp@lists.linux.it
Cc: andrea.cervesato@suse.com, pvorel@suse.cz
Subject: Re: [PATCH 1/3] doc: Add missing API references to api_c_tests.rst

Hi Andrea,

Series looks good overall — the reorganisation, kernel-doc conversions and
the new SAFE_* reference page are all useful additions. A few issues to fix
before merging:

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

Commit message: empty "Verdict:" trailer
-----------------------------------------

The commit message contains a bare "Verdict:" line with no value:

  Fixes: eef750db28 ("libs: adopt tse_ prefix for extended (non-core) library")
  Reviewed-by: Petr Vorel <pvorel@suse.cz>
  Verdict:
  Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>

This looks like a leftover placeholder from a review-template tool.
Please remove the "Verdict:" line before sending v2.

include/tst_kconfig.h: multi-line kernel-doc summary
------------------------------------------------------

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

Kernel-doc requires the one-line summary to fit on the same line as the
identifier, i.e. everything between " - " and the first blank comment
line is treated as the description. A line-wrapped summary like the above
is not valid; the second line will not be merged into the summary by the
extractor. Fix by combining the description onto a single line, or move
the "Requires <string.h>" note into the description body after a blank:

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

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

SAFE_GETGRNAM_FALLBACK — missing argument
-----------------------------------------

> - ``SAFE_GETGRNAM_FALLBACK(name)``

The actual macro in include/tst_safe_macros.h takes two arguments:

  #define SAFE_GETGRNAM_FALLBACK(name, fallback) \

Please correct to:

  ``SAFE_GETGRNAM_FALLBACK(name, fallback)``

SAFE_MOUNT2 — missing is_fuse argument
---------------------------------------

> - ``SAFE_MOUNT2(source, target, filesystemtype, mountflags, data)``

The macro in include/tst_safe_macros.h takes a sixth argument:

  #define SAFE_MOUNT2(source, target, filesystemtype, \
                      mountflags, data, is_fuse) \

Please correct to:

  ``SAFE_MOUNT2(source, target, filesystemtype, mountflags, data, is_fuse)``

SAFE_IO_URING_ENTER — missing strict argument
---------------------------------------------

> - ``SAFE_IO_URING_ENTER(fd, to_submit, min_complete, flags, sig)``

The macro in include/tst_safe_io_uring.h takes strict as its first argument:

  #define SAFE_IO_URING_ENTER(strict, fd, to_submit, min_complete, flags, sig) \

Please correct to:

  ``SAFE_IO_URING_ENTER(strict, fd, to_submit, min_complete, flags, sig)``

Commit message: "all 245" vs actual macro count ⚠️
----------------------------------------------------

The commit message says "listing all 245 SAFE_* wrapper macros", but
grepping the relevant headers finds approximately 247 #define SAFE_*
entries. If a small number of macros are intentionally excluded (e.g.
internal or deprecated ones), please either update the count or note
which macros are omitted and why.

---
Everything else in the series looks correct:

[PATCH 1/3]
  - The 10 headers listed in the commit message match the diff.
  - Alphabetical re-ordering in api_c_tests.rst is correct.
  - Kernel-doc conversions in tse_uinput.h, tst_clone.h, tst_device.h,
    tst_fuzzy_sync.h, tst_hugepage.h, tst_taint.h are accurate and match
    the implementations (tst_clear_device() is confirmed called from
    tst_mkfs(), tst_taint_check() auto-called at end of test, etc.).

[PATCH 2/3]
  - The 8 missing .tst_test fields are correctly identified and added.
  - .mnt_flags / .min_swap_avail alphabetical fix is correct.
  - Shell API variables section matches tst_test.sh.

[PATCH 3/3]
  - SAFE_MKFS in tst_mkfs.h: confirmed.
  - SAFE_FORK in tst_test.h, SAFE_CLONE in tst_clone.h: confirmed.
  - SAFE_CG_* in tst_cgroup.h, SAFE_EPOLL_* in tst_epoll.h: confirmed.
  - SAFE_RT_SIGACTION / SAFE_RT_SIGPROCMASK in lapi/safe_rt_signal.h: confirmed.

Please address the five issues above (four wrong macro signatures / one
stray commit message line) in v2.

Thanks,
Copilot reviewer

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