Re: [LTP] [PATCH v8 4/4] Add documentation about syscalls.h generator

[Petr Vorel <pvorel@suse.cz>]

HI Andrea, Cyril,

> > +++ b/doc/developers/test_case_tutorial.rst
> > @@ -302,24 +302,25 @@ for a distribution's C library version to be older than its kernel or it may use
> >  cut down C library in comparison to the GNU one. So we must call ``statx()``
> >  using the general ``syscall()`` interface.

> > -The LTP contains a library for dealing with the ``syscall`` interface, which is
> > +LTP contains a library for dealing with the ``syscall`` interface, which is
> >  located in :master:`include/lapi`. System call numbers are listed against the relevant
> >  call in the ``*.in`` files (e.g. ``x86_64.in``) which are used to generate
> > -``syscalls.h``, which is the header you should include. On rare occasions you
> > -may find the system call number is missing from the ``*.in`` files and will need
> > -to add it (see :master:`include/lapi/syscalls/strip_syscall.awk`).
> > +``syscalls.h``, the header you should include.

> > -System call numbers vary between architectures, hence there are multiple
> > -``*.in`` files for each architecture.

> I would keep this sentence in there, probably as a last sentence in the
> upper paragraph.

+1

> > - You can find the various values for the
> > -``statx`` system call across a number of ``unistd.h`` files in the Linux kernel.
> > +On rare occasions, you may find that system call number is missing from ``*.in``
> > +files. In these cases, they will need to be updated using
> > +`include/lapi/syscalls/generate_arch.sh` script as following:
>         ^
> 	Just `generate_arch.sh` here, we list the full path in the code
> 	block, no need to repeat it here as well.
+1. BTW it should be either ``generate_arch.sh`` (double `` is used for
other commands, e.g. ``make check`` or ``./configure``) or it could be a link to
a source:

:master:`include/lapi/syscalls/generate_arch.sh` script as following:

I like links, because reader can see the script content. Also it can help to
verify whether the script is still at the repository (was not moved/deleted).

> > -Note that we don't use the system-call-identifier value available in
> > -``/usr/include/linux/uinstd.h`` because the kernel might be much newer than the
> > -user land development packages.
> > +.. code-block:: bash
> > +
> > +    $ include/lapi/syscalls/generate_arch.sh /path/of/linux/sources
>                                                       ^
> 						      to

very nit: "path/to/Linux/kernel/sources" (upper "L", which you use later, also
it's used in the script).

> > +The script will generate all the needed ``*.in`` files according to the Linux
>                                                             ^
> 							  accordingly
+1
> > +source code which has been used. Make sure that your Linux source code has
> > +been updated to the latest version.

> > -For ``statx`` we had to add ``statx 332`` to :master:`include/lapi/syscalls/x86_64.in`,
> > -``statx 383`` to :master:`include/lapi/syscalls/powerpc.in`, etc.  Now lets look at
> > -the code, which I will explain in more detail further down.
> > +Once the new syscalls files have been updated, to rebuild our ``syscalls.h``
> > +file, please use ``./configure`` command.
>             ^                         ^
> 	 re-run			      script
+1

Otherwise LGTM, thanks a lot for bothering with the docs.

Kind regards,
Petr

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