Re: [LTP] [PATCH 1/4] doc: Fix broken references and outdated content

[Petr Vorel <pvorel@suse.cz>]

Hi Andrea,

> Fix kernel-doc reference to renamed tse_numa.h header, correct
> TST_EXP_POSSITIVE typo and RST formatting in tst_test_macros.h,
> use :c:struct: instead of :ref: for struct cross-references, fix
> broken lapi/ :master: links, drop stale lapi/stat.h notes, correct
> kirk invocation example and use md5sum/sha1sum commands.

Maybe add:

Fixes: eef750db28 ("libs: adopt tse_ prefix for extended (non-core) library")

(there are more fixes but this one is the most obvious.)

And I'm sorry I touched the docs (you might need to resolve conflict during
rebasing), I haven't noticed this patchset otherwise I'd wait.

> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  doc/developers/api_c_tests.rst            |  2 +-
>  doc/developers/api_shell_tests.rst        |  2 +-
>  doc/developers/test_case_tutorial.rst     | 19 ++++++++-----------
>  doc/developers/todo.rst                   |  2 +-
>  doc/developers/writing_tests.rst          |  6 +++---
>  doc/maintainers/ltp_release_procedure.rst |  4 ++--
>  doc/maintainers/patch_review.rst          |  6 +++---
>  include/tst_test_macros.h                 |  8 ++++----
>  8 files changed, 23 insertions(+), 26 deletions(-)

> diff --git a/doc/developers/api_c_tests.rst b/doc/developers/api_c_tests.rst
> index 26b46d92d9f0aa724f10a960b6c5cce59ae1d3e8..65cf6d6cd0be5cb4cd9c986e09df021071d968f1 100644
> --- a/doc/developers/api_c_tests.rst
> +++ b/doc/developers/api_c_tests.rst
> @@ -50,7 +50,7 @@ Process state

>  NUMA
>  ----
> -.. kernel-doc:: ../../include/tst_numa.h
> +.. kernel-doc:: ../../include/tse_numa.h

>  Option parsing
>  --------------
> diff --git a/doc/developers/api_shell_tests.rst b/doc/developers/api_shell_tests.rst
> index 51bb04a4e155e83c3302b4664886d49624f51a9f..9a3ff795f1b0cedbfa3b126fe0acad8d24e5c704 100644
> --- a/doc/developers/api_shell_tests.rst
> +++ b/doc/developers/api_shell_tests.rst
> @@ -30,7 +30,7 @@ documentation at https://linux-test-project.readthedocs.io/en/latest/users/test_
>  The shell loader test library uses the :doc:`../developers/api_c_tests`
>  internally by parsing a special JSON formatted comment and
>  initializing it accordingly. The JSON format is nearly 1:1 serialization of the
> -:ref:`struct tst_test` into a JSON. The environment must be always preset even
> +:c:struct:`tst_test` into a JSON. The environment must be always preset even
>  when it's empty.

OK, *.rst files require to use :c:struct: instead of :ref:. It's probably my
error (I haven't noticed the syntax is different from C files).

>  .. code-block:: shell
> diff --git a/doc/developers/test_case_tutorial.rst b/doc/developers/test_case_tutorial.rst
> index 240897e36b8fd7c5f70cf426f81ef4bfc0e562a0..28eda5ae69f7e358383ae44d5d23bd2e4d79b487 100644
> --- a/doc/developers/test_case_tutorial.rst
> +++ b/doc/developers/test_case_tutorial.rst
> @@ -137,7 +137,7 @@ initialization code. It is important to note that this is a completely
>  ordinary, independent C program, however ``main()`` is missing because it is
>  implemented in :master:`include/tst_test.h`.

> -We specify what code we want to run as part of the test using :ref:`struct tst_test`.
> +We specify what code we want to run as part of the test using :c:struct:`tst_test`.
>  Various callbacks can be set by the test writer, including
>  ``test.test_all``, which we have set to ``run()``. The test harness will execute
>  this callback in a separate process (using ``fork()``), forcibly terminating it
> @@ -205,8 +205,8 @@ please do:
nit: could you please use :manpage:`fork()` ?

>  This should build the test and then run it. However, even though the test is
>  in :master:`testcases/kernel/syscalls` directory it won't be automatically ran
> -as part of the syscalls test group (e.g. not run via ``kirk -r math``.  For
> -this we need to add it to the runtest file. So open :master:`runtest/syscalls`
> +as part of the syscalls test group (e.g. not run via ``kirk -f syscalls``).
> +For this we need to add it to the runtest file. So open :master:`runtest/syscalls`
IMHO math is fast => good as example.

>  and add the lines starting with a ``+``.

>  .. code-block::
> @@ -517,15 +517,14 @@ that should be by setting ``.needs_tmpdir = 1``.
>      /*************** statx structure and wrapper goes here ! ***************/
>      ...

> -We have added an extra include :master:`lapi/fcntl.h` which wraps the system header by
> +We have added an extra include :master:`include/lapi/fcntl.h` which wraps the system header by
>  the same name (``#include <fcntl.h>``). This header ensures we have definitions
>  for recently added macros such as ``AT_FDCWD`` by providing fall backs if the
> -system header does not have them. The :master:`lapi/` directory contains a number of
> +system header does not have them. The :master:`include/lapi/` directory contains a number of
>  headers like this.
+1

> -At some point we may wish to add :master:`lapi/stat.h` to provide a fall back for
> -macros such as ``STATX_BASIC_STATS``. However for the time being we have just
> -defined it in the test.
> +The :master:`include/lapi/stat.h` header provides similar fall backs for
> +macros such as ``STATX_BASIC_STATS`` used by this test.
+1

...
>  Shell coding style
> @@ -361,7 +361,7 @@ LTP C And Shell Test API Comparison
>  .. list-table::
>      :header-rows: 1

> -    * - C API :ref:`struct tst_test` members
> +    * - C API :c:struct:`tst_test` members
>        - Shell API ``$TST_*`` variables

>      * - .all_filesystems
> @@ -531,5 +531,5 @@ LTP C And Shell Test API Comparison
>      * - C API other structs
>        - Shell API ``$TST_*`` variables

> -    * - :ref:`struct tst_device`
> +    * - ``struct tst_device``
I'd even add
:c:struct:`struct tst_device` as a preparation for a new commit but you might
add it in the following patchset.


>        - TST_DEVICE
> diff --git a/doc/maintainers/ltp_release_procedure.rst b/doc/maintainers/ltp_release_procedure.rst
> index 6dbafa4f81ebd3c843ef66547340012c59521fb8..7851440125ba54707026d3d380e2b3ef0def368d 100644
> --- a/doc/maintainers/ltp_release_procedure.rst
> +++ b/doc/maintainers/ltp_release_procedure.rst
> @@ -112,8 +112,8 @@ metadata documentation:
>      tar -cJf ltp-full-YYYYMMDD.tar.xz ltp-full-YYYYMMDD --exclude .git

>      # Generate checksums
> -    md5 ltp-full-YYYYMMDD.tar.xz > ltp-full-YYYYMMDD.tar.xz.md5
> -    sha1 ltp-full-YYYYMMDD.tar.xz > ltp-full-YYYYMMDD.tar.xz.sha1
> +    md5sum    ltp-full-YYYYMMDD.tar.xz > ltp-full-YYYYMMDD.tar.xz.md5
> +    sha1sum   ltp-full-YYYYMMDD.tar.xz > ltp-full-YYYYMMDD.tar.xz.sha1
+1
>      sha256sum ltp-full-YYYYMMDD.tar.xz > ltp-full-YYYYMMDD.tar.xz.sha256

>  You can use :master:`tools/create-tarballs-metadata.sh` script to have the above
> diff --git a/doc/maintainers/patch_review.rst b/doc/maintainers/patch_review.rst
> index 28bc4faf343047edfcf647b38f053f923b7de5c7..a328948094029c2a6c2b550c66df8e0236c0ab42 100644
> --- a/doc/maintainers/patch_review.rst
> +++ b/doc/maintainers/patch_review.rst
> @@ -146,12 +146,12 @@ New test should
>  C tests
>  ~~~~~~~

> -* Use :doc:`../developers/api_c_tests`, implementing :ref:`struct tst_test`
> +* Use :doc:`../developers/api_c_tests`, implementing :c:struct:`tst_test`
+1

>  * Test binaries are added into corresponding ``.gitignore`` files
>  * Check coding style with ``make check``
>  * Metadata documentation
> -* If a test is a regression test it should include :ref:`.tags<struct tst_tag>` in the
> -  :ref:`struct tst_test` definition
> +* If a test is a regression test it should include :c:struct:`.tags <tst_tag>` in the
> +  :c:struct:`tst_test` definition

>  Shell tests
>  ~~~~~~~~~~~
> diff --git a/include/tst_test_macros.h b/include/tst_test_macros.h
> index b1e74278d5ddb25293d38507482dc5a5cdafdbd0..f06c8aeb77558c0c09880f905ccc5b85d9f277a3 100644
> --- a/include/tst_test_macros.h
> +++ b/include/tst_test_macros.h
> @@ -146,7 +146,7 @@ extern int TST_PASS;
>   * @SCALL: Tested syscall.
>   * @...: A printf-like parameters.
>   *
> - * This is a variant of the TST_EXP_POSSITIVE() for a more specific case that
> + * This is a variant of the TST_EXP_POSITIVE() for a more specific case that
>   * the returned value is a file descriptor.
>   */
>  #define TST_EXP_FD(SCALL, ...)                                                 \
> @@ -200,7 +200,7 @@ extern int TST_PASS;
>   * @SCALL: Tested syscall.
>   * @...: A printf-like parameters.
>   *
> - * This is a variant of the TST_EXP_POSSITIVE() for a more specific case that
> + * This is a variant of the TST_EXP_POSITIVE() for a more specific case that
>   * the returned value is a pid.
>   */
>  #define TST_EXP_PID(SCALL, ...)                                                \
> @@ -379,8 +379,8 @@ extern int TST_PASS;
>   * @...: A printf-like parameters.
>   *
>   * This macro works like TST_EXP_PASS_PTR_VOID() but checks the return
> - * value against NULL instead of (void *)-1. Use this for libc functions
> - * such as fopen() that return NULL on failure.
> + * value against NULL instead of ``(void *)-1``. Use this for libc functions
> + * such as :manpage:`fopen(3)` that return NULL on failure.
+1

>   */
>  #define TST_EXP_PASS_PTR_NULL(SCALL, ...)                                      \
>  	TST_EXP_PASS_PTR_(SCALL, #SCALL, NULL, ##__VA_ARGS__)

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

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