xref: /aosp_15_r20/external/ltp/doc/developers/test_case_tutorial.rst (revision 49cdfc7efb34551c7342be41a7384b9c40d7cab7)
1.. SPDX-License-Identifier: GPL-2.0-or-later
2
3Test case tutorial
4==================
5
6This is a step-by-step tutorial on writing a simple C LTP test, where topics
7of the LTP and Linux kernel testing will be introduced gradually using a
8concrete example. Most sections will include exercises, some trivial and
9others not so much. If you find an exercise is leading you off at too much of
10a tangent, just leave it for later and move on.
11
12LTP tests can be written in C or Shell script. This tutorial is **only for tests
13written in C** using the new LTP test API. Note that while we go into some
14detail on using Git, this is not intended as a canonical or complete guide
15for Git.
16
17Assumptions & Feedback
18----------------------
19
20We assume the reader is familiar with C, Git and common Unix/Linux/GNU tools
21and has some general knowledge of Operating Systems. Experienced Linux
22developers may find it too verbose while people new to system level Linux
23development may find it overwhelming.
24
25Comments and feedback are welcome, please direct them to the Mailing list.
26
27Getting Started
28---------------
29
30First of all, make sure you have a copy of LTP in the current folder
31and we recommended cloning the Linux kernel repository for reference, this
32guide will refer to files and directories.
33
34.. code-block:: bash
35
36    git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
37
38There are a number of other repositories which are useful for reference as
39well, including the GNU C library ``glibc`` and the alternative C library
40``musl``. Some system calls are partially or even entirely implemented in user
41land as part of the standard C library. So in these cases, the C library is an
42important reference. ``glibc`` is the most common C library for Linux, however
43``musl`` is generally easier to understand.
44
45How system calls are implemented varies from one architecture to another and
46across kernel and C library versions. To find out whether a system call is
47actually accessing the kernel (whether it is actually a system call) on any
48given machine you can use the ``strace`` utility. This intercepts system calls
49made by an executable and prints them. We will use this later in the tutorial.
50
51Choose a System Call to test
52----------------------------
53
54We will use the ``statx()`` system call, to provide a concrete example of a
55test. At the time of writing there is no test for this call which was
56introduced in Linux kernel version 4.11.
57
58Linux system call specific tests are primarily contained in
59:master:`testcases/kernel/syscalls`, but you should also :git_man:`grep` the
60entire LTP repository to check for any existing usages of a system call.
61
62One way to find a system call which is not currently tested by the LTP is to
63look at :kernel_tree:`include/linux/syscalls.h` in the Linux kernel tree.
64
65Something the LTP excels to ensure bug-fixes are back ported to
66maintenance releases, so targeting a specific regression is another
67option.
68
69Find an untested System call
70~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
71
72Try to find an untested system call which has a manual page (i.e. ``man
73syscall`` produces a result). It is a good idea to Git-clone the latest kernel
74man-pages repository.
75
76.. code-block:: bash
77
78    git clone git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
79
80At the time of writing, the difference between the latest man-pages release and
81the ``HEAD`` of the repository (usually the latest commit) is well over 100
82commits. This represents about 9 weeks of changes. If you are using a stable
83Linux distribution, your man-pages package may well be years old. So as with
84the kernel, it is best to have the Git repository as a reference.
85
86You could also find a system call with untested parameters or use whatever it
87is you are planning to use the LTP for.
88
89Create the test skeleton
90------------------------
91
92I shall call my test ``statx01.c``, by the time you read this that file name
93will probably be taken, so increment the number in the file name as
94appropriate or replace ``statx`` with the system call in the chosen exercise.
95
96.. code-block:: bash
97
98    mkdir testcases/kernel/syscalls/statx
99    cd testcases/kernel/syscalls/statx
100    echo statx >> .gitignore
101
102Next open ``statx01.c`` and add the following boilerplate. Make sure to change
103the copyright notice to your name/company, correct the test name and minimum
104kernel version if necessary. I will explain what the code does below.
105
106.. code-block:: c
107
108    // SPDX-License-Identifier: GPL-2.0-or-later
109    /*
110    * Copyright (c) 2017 Instruction Ignorer <"can't"@be.bothered.com>
111    */
112
113    /*\
114    * [Description]
115    *
116    * All tests should start with a description of _what_ we are testing.
117    * Non-trivial explanations of _how_ the code works should also go here.
118    * Include relevant links, Git commit hashes and CVE numbers.
119    * Inline comments should be avoided.
120    */
121
122    #include "tst_test.h"
123
124    static void run(void)
125    {
126        tst_res(TPASS, "Doing hardly anything is easy");
127    }
128
129    static struct tst_test test = {
130        .test_all = run,
131        .min_kver = "4.11",
132    };
133
134Starting with the ``#include`` statement we copy in the main LTP test library
135headers. This includes the most common test API functions and the test harness
136initialization code. It is important to note that this is a completely
137ordinary, independent C program, however ``main()`` is missing because it is
138implemented in ``tst_test.h``.
139
140We specify what code we want to run as part of the test using the ``tst_test
141test`` structure. Various callbacks can be set by the test writer, including
142``test.test_all``, which we have set to ``run()``. The test harness will execute
143this callback in a separate process (using ``fork()``), forcibly terminating it
144if it does not return after ``test.timeout`` seconds.
145
146We have also set ``test.min_kver`` to the kernel version where ``statx`` was
147introduced. The test library will determine the kernel version at runtime. If
148the version is less than 4.11 then the test harness will return ``TCONF``,
149indicating that this test is not suitable for the current system
150configuration.
151
152Occasionally features are back ported to older kernel versions, so ``statx`` may
153exist on kernels with a lower version. However we don't need to worry about
154that unless there is evidence of it happening.
155
156As mentioned in the code itself, you should specify what you are testing and
157the expected outcome, even if it is relatively simple. If your program flow is
158necessarily complex and difficult to understand (which is often the case when
159trying to manipulate the kernel into doing something bad), then a detailed
160explanation of how the code works is welcome.
161
162What you should not do, is use inline comments or include the same level of
163explanation which is written here. As a general rule, if something is easy to
164document, then the code should also be easy to read. So don't document the easy
165stuff (except for the basic test specification).
166
167Before continuing we should compile this and check that the basics work. In
168order to compile the test we need a ``Makefile`` in the same subdirectory. If
169one already exists, then nothing needs to be done, otherwise add one with the
170following contents.
171
172.. code-block:: make
173
174    # SPDX-License-Identifier: GPL-2.0-or-later
175    # Copyright (c) 2019 Linux Test Project
176
177    top_srcdir		?= ../../../..
178
179    include $(top_srcdir)/include/mk/testcases.mk
180
181    include $(top_srcdir)/include/mk/generic_leaf_target.mk
182
183This will automatically add ``statx01.c`` as a build target producing a
184``statx01`` executable. Unless you have heavily deviated from the tutorial, and
185probably need to change ``top_srcdir``, nothing else needs to be done.
186
187Normally, if you were starting a Makefile from scratch, then you would need to
188add ``statx01`` as a build target. Specifying that you would like to run some
189program (e.g. ``gcc`` or ``clang``) to transform ``statx01.c`` into ``statx01``.
190Here we don't need to do that, but sometimes it is still necessary. For example,
191if we needed to link to the POSIX threading library, then we could add the
192following line after ``testcases.mk``.
193
194.. code-block:: make
195
196    statx01: CFLAGS += -pthread
197
198Assuming you are in the test's subdirectory :master:`testcases/kernel/syscalls/statx`,
199please do:
200
201.. code-block:: bash
202
203    make
204    ./statx01
205
206This should build the test and then run it. However, even though the test is
207in :master:`testcases/kernel/syscalls` directory it won't be automatically ran
208as part of the syscalls test group (e.g. not run via ``kirk -r math`` or
209``./runltp -f syscalls``). For this we need to add it to the runtest file. So
210open :master:`runtest/syscalls` and add the lines starting with a ``+``.
211
212.. code-block::
213
214    statvfs01 statvfs01
215    statvfs02 statvfs02
216
217    +statx01 statx01
218    +
219    stime01 stime01
220    stime02 stime02
221
222The :master:`runtest` files are in a two column format. The first column is the
223test name, which is mainly used by test runners for reporting and filtering. It
224is just a single string of text with no spaces. The second column, which can
225contain spaces, is passed to the shell in order to execute the test. Often it
226is just the executable name, but some tests also take arguments (the LTP has a
227library for argument parsing, by the way).
228
229If you haven't done so already, we should add all these new files to Git. It
230is vitally important that you do not make changes to the master branch. If you
231do then pulling changes from upstream becomes a major issue. So first of all
232create a new branch.
233
234.. code-block:: bash
235
236    git checkout -b statx01 master
237
238Now we want to add the files we have created or modified, but before doing a
239commit make sure you have configured Git correctly. You need to at least set
240your Name and e-mail address in ``~/.gitconfig``, but there are some other
241settings which come in handy too. My relatively simple configuration is similar
242to the below:
243
244.. code-block:: ini
245
246    [user]
247        name = Sarah Jane
248        email = [email protected]
249    [core]
250        editor = emacs
251    [sendemail]
252        smtpServer = smtp.server.address
253
254Obviously you need to at least change your name and e-mail. The SMTP server is
255useful for :git_man:`send-email`, which we will discuss later. The editor value is
256used for things like writing commits (without the ``-m`` option).
257
258.. code-block:: bash
259
260    git add -v :/testcases/kernel/syscalls/statx :/runtest/syscalls
261    git commit -m "statx01: Add new test for statx syscall"
262
263This should add all the new files in the ``statx`` directory and the ``runtest``
264file. It is good practice to commit early and often. Later on we will do a
265Git-rebase, which allows us to clean up the commit history. So don't worry
266about how presentable your commit log is for now. Also don't hesitate to
267create a new branch when doing the exercises or experimenting. This will allow
268you to diverge from the tutorial and then easily come back again.
269
270I can't emphasize enough that Git makes things easy through branching and that
271things quickly get complicated if you don't do it. However if you do get into
272a mess, Git-reflog and Git-reset, will usually get you out of it. If you also
273mess that up then it may be possible to cherry pick 'dangling' commits out of
274the database into a branch.
275
276Report TCONF instead of TPASS
277~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
278
279Maybe the test should report ``TCONF: Not implemented`` instead or perhaps
280``TBROK``. Try changing it do so.
281
282Check Git ignores the executable
283~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
284
285Is your ``.gitignore`` correct?
286
287Run make check
288~~~~~~~~~~~~~~~~~~
289
290Check coding style with ``make check``.
291
292Install the LTP and run the test with runtest
293~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
294
295Run ``statx01`` on its own, also using ``-I0`` amd ``-I10``.
296
297Call the system call
298--------------------
299
300At the time of writing ``statx`` has no ``glibc`` wrapper. It is also fairly common
301for a distribution's C library version to be older than its kernel or it may use a
302cut down C library in comparison to the GNU one. So we must call ``statx()``
303using the general ``syscall()`` interface.
304
305The LTP contains a library for dealing with the ``syscall`` interface, which is
306located in :master:`include/lapi`. System call numbers are listed against the relevant
307call in the ``*.in`` files (e.g. ``x86_64.in``) which are used to generate
308``syscalls.h``, which is the header you should include. On rare occasions you
309may find the system call number is missing from the ``*.in`` files and will need
310to add it (see :master:`include/lapi/syscalls/strip_syscall.awk`).
311
312System call numbers vary between architectures, hence there are multiple
313``*.in`` files for each architecture. You can find the various values for the
314``statx`` system call across a number of ``unistd.h`` files in the Linux kernel.
315
316Note that we don't use the system-call-identifier value available in
317``/usr/include/linux/uinstd.h`` because the kernel might be much newer than the
318user land development packages.
319
320For ``statx`` we had to add ``statx 332`` to :master:`include/lapi/syscalls/x86_64.in`,
321``statx 383`` to :master:`include/lapi/syscalls/powerpc.in`, etc.  Now lets look at
322the code, which I will explain in more detail further down.
323
324.. code-block:: c
325
326    /*
327    * Test statx
328    *
329    * Check if statx exists and what error code it returns when we give it dodgy
330    * data.
331    */
332
333    #include <stdint.h>
334    #include "tst_test.h"
335    #include "lapi/syscalls.h"
336
337    struct statx_timestamp {
338        int64_t	       tv_sec;
339        uint32_t       tv_nsec;
340        int32_t	       __reserved;
341    };
342
343    struct statx {
344        uint32_t	stx_mask;
345        uint32_t	stx_blksize;
346        uint64_t	stx_attributes;
347        uint32_t	stx_nlink;
348        uint32_t	stx_uid;
349        uint32_t	stx_gid;
350        uint16_t	stx_mode;
351        uint16_t	__spare0[1];
352        uint64_t	stx_ino;
353        uint64_t	stx_size;
354        uint64_t	stx_blocks;
355        uint64_t	stx_attributes_mask;
356        struct statx_timestamp	stx_atime;
357        struct statx_timestamp	stx_btime;
358        struct statx_timestamp	stx_ctime;
359        struct statx_timestamp	stx_mtime;
360        uint32_t	stx_rdev_major;
361        uint32_t	stx_rdev_minor;
362        uint32_t	stx_dev_major;
363        uint32_t	stx_dev_minor;
364        uint64_t	__spare2[14];
365    };
366
367    static int sys_statx(int dirfd, const char *pathname, int flags,
368                unsigned int mask, struct statx *statxbuf)
369    {
370        return tst_syscall(__NR_statx, dirfd, pathname, flags, mask, statxbuf);
371    }
372
373    ...
374
375So the top part of the code is now boiler plate for calling ``statx``. It is
376common for the kernel to be newer than the user land libraries and headers. So
377for new system calls like ``statx``, we copy, with a few modifications, the
378relevant definitions into the LTP. This is somewhat like 'vendoring', although
379we are usually just copying headers required for interacting with the Kernel's
380ABI (Application Binary Interface), rather than integrating actual
381functionality.
382
383So from the top we include the ``stdint.h`` library which gives us the standard
384``(u)int*_t`` type definitions. We use these in place of the Kernel type
385definitions such as ``__u64`` in ``linux/types.h``. We then have a couple of
386structure definitions which form part of the ``statx`` API. These were copied
387from :kernel_tree:`include/uapi/linux/stat.h` in the Linux kernel tree.
388
389After that, there is a wrapper function, which saves us from writing
390``tst_syscall(__NR_statx, ...``, every time we want to make a call to
391``statx``. This also provides a stub for when ``statx`` is eventually integrated
392into the LTP library and also implemented by the C library. At that point we
393can switch to using the C library implementation if available or fallback to
394our own.
395
396The advantage of using the C library implementation is that it will often be
397better supported across multiple architectures. It will also mean we are using
398the system call in the same way most real programs would. Sometimes there are
399advantages to bypassing the C library, but in general it should not be our
400first choice.
401
402The final test should do a check during configuration (i.e. when we run
403``./configure`` before building) which checks if the ``statx`` system call and
404associated structures exists. This requires writing an ``m4`` file for use with
405:master:`configure.ac` which is processed during ``make autotools`` and produces the
406configure script.
407
408For the time being though we shall just ignore this. All you need to know for
409now is that this is a problem which eventually needs to be dealt with and that
410there is a system in place to handle it.
411
412.. code-block:: c
413
414    ...
415
416    static void run(void)
417    {
418        struct statx statxbuf = { 0 };
419
420        TEST(sys_statx(0, NULL, 0, 0, &statxbuf));
421
422        if (TST_RET == 0)
423            tst_res(TFAIL, "statx thinks it can stat NULL");
424        else if (TST_ERR == EFAULT)
425            tst_res(TPASS, "statx set errno to EFAULT as expected");
426        else
427            tst_res(TFAIL | TERRNO, "statx set errno to some unexpected value");
428    }
429
430    static struct tst_test test = {
431        .test_all = run,
432        .min_kver = "4.11",
433    };
434
435The ``TEST`` macro sets ``TST_RET`` to the return value of ``tst_statx()`` and
436``TST_ERR`` to the value of ``errno`` immediately after the functions
437return. This is mainly just for convenience, although it potentially could
438have other uses.
439
440We check whether the return value indicates success and if it doesn't also
441check the value of ``errno``. The last call to ``tst_res`` includes ``TERRNO``,
442which will print the current error number and associated description in
443addition to the message we have provided. Note that it uses the current value
444of ``errno`` not ``TST_ERR``.
445
446What we should have done in the example above is use ``TTERRNO`` which takes the
447value of ``TST_ERR``.
448
449If we try to run the test on a kernel where ``statx`` does not exist, then
450``tst_syscall`` will cause it to fail gracefully with ``TCONF``. Where ``TCONF``
451indicates the test is not applicable to our configuration.
452
453The function ``tst_syscall`` calls ``tst_brk(TCONF,...)`` on failure. ``tst_brk``
454causes the test to exit immediately, which prevents any further test code from
455being run.
456
457What are the differences between ``tst_brk`` and ``tst_res``?
458~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
459
460See :master:`include/tst_test.h`. Also what do they have in common?
461
462What happens if you call ``tst_res(TINFO, ...)`` after ``sys_statx``?
463~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
464
465Does the test still function correctly?
466
467Extend the test to handle other basic error conditions
468~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
469
470For example, see if you can trigger ``ENOENT`` instead. You shouldn't
471have to create any files, which is discussed in the next section.
472
473Setup, Cleanup and files
474------------------------
475
476Some tests require resources to be allocated, or system settings to be
477changed, before the test begins. This ``setup`` only has to be done once at the
478beginning and at the end of the test needs to be removed or reverted. The
479``cleanup`` also has to be done regardless of whether the test breaks.
480
481Fortunately, like most test libraries, we have setup and cleanup (teardown)
482callbacks. ``setup`` is called once before ``run`` and ``cleanup`` is called once
483afterwards. Note that ``run`` itself can be called multiple times by the test
484harness, but that ``setup`` and ``cleanup`` are only called once.
485
486If either your code, a ``SAFE_*`` macro or a library function such as
487``tst_syscall`` call ``tst_brk``, then ``run`` will exit immediately and the
488``cleanup`` function is then called. Once ``cleanup`` is completed, the test
489executable will then exit altogether abandoning any remaining iterations of
490``run``.
491
492For ``statx`` we would like to create some files or file like objects which we
493have control over. Deciding where to create the files is easy, we just create
494it in the current working directory and let the LTP test harness handle where
495that should be by setting ``.needs_tmpdir = 1``.
496
497.. code-block:: c
498
499    /*
500    * Test statx
501    *
502    * Check if statx exists and what error code it returns when we give it dodgy
503    * data. Then stat a file and check it returns success.
504    */
505
506    #include <stdint.h>
507    #include "tst_test.h"
508    #include "lapi/syscalls.h"
509    #include "lapi/fcntl.h"
510
511    #define FNAME "file_to_stat"
512    #define STATX_BASIC_STATS 0x000007ffU
513
514    /*************** statx structure and wrapper goes here ! ***************/
515    ...
516
517We have added an extra include :master:`lapi/fcntl.h` which wraps the system header by
518the same name (``#include <fcntl.h>``). This header ensures we have definitions
519for recently added macros such as ``AT_FDCWD`` by providing fall backs if the
520system header does not have them. The :master:`lapi/` directory contains a number of
521headers like this.
522
523At some point we may wish to add :master:`lapi/stat.h` to provide a fall back for
524macros such as ``STATX_BASIC_STATS``. However for the time being we have just
525defined it in the test.
526
527
528.. code-block:: c
529
530    ...
531
532    static void setup(void)
533    {
534        SAFE_TOUCH(FNAME, 0777, NULL);
535    }
536
537    static void run(void)
538    {
539        struct statx statxbuf = { 0 };
540
541        TEST(sys_statx(0, NULL, 0, 0, &statxbuf));
542        if (TST_RET == 0)
543            tst_res(TFAIL, "statx thinks it can stat NULL");
544        else if (TST_ERR == EFAULT)
545            tst_res(TPASS, "statx set errno to EFAULT as expected");
546        else
547            tst_res(TFAIL | TERRNO, "statx set errno to some unexpected value");
548
549        TEST(sys_statx(AT_FDCWD, FNAME, 0, STATX_BASIC_STATS, &statxbuf));
550        if (TST_RET == 0)
551            tst_res(TPASS, "It returned zero so it must have worked!");
552        else
553            tst_res(TFAIL | TERRNO, "statx can not stat a basic file");
554    }
555
556    static struct tst_test test = {
557        .setup = setup,
558        .test_all = run,
559        .min_kver = "4.11",
560        .needs_tmpdir = 1
561    };
562
563The ``setup`` callback uses one of the LTP's ``SAFE`` functions to create an empty
564file ``file_to_stat``. Because we have set ``.needs_tmpdir``, we can just create
565this file in the present working directory. We don't need to create a
566``cleanup`` callback yet because the LTP test harness will recursively delete
567the temporary directory and its contents.
568
569The ``run`` function can be called multiple times by the test harness, however
570``setup`` and ``cleanup`` callbacks will only be ran once.
571
572.. warning::
573
574    By this point you may have begun to explore the LTP library headers or older
575    tests. In which case you will have come across functions from the old API such
576    as ``tst_brkm``. The old API is being phased out, so you should not use these
577    functions.
578
579So far we haven't had to do any clean up. So our example doesn't answer the
580question "what happens if part of the clean up fails?". To answer this we are
581going to modify the test to ask the (highly contrived) question "What happens
582if I create and open a file, then create a hard-link to it, then call open
583again on the hard-link, then ``stat`` the file".
584
585
586.. code-block:: c
587
588    #define LNAME "file_to_stat_link"
589
590    ...
591
592    static void setup(void)
593    {
594        fd = SAFE_OPEN(FNAME, O_CREAT, 0777);
595        SAFE_LINK(FNAME, LNAME);
596        lfd = SAFE_OPEN(LNAME, 0);
597    }
598
599    static void cleanup(void)
600    {
601        if (lfd != 0)
602            SAFE_CLOSE(lfd);
603
604        if (fd != 0)
605            SAFE_CLOSE(fd);
606    }
607
608    static void run(void)
609    {
610            ...
611
612        TEST(sys_statx(AT_FDCWD, LNAME, 0, STATX_BASIC_STATS, &statxbuf));
613        if (TST_RET == 0)
614            tst_res(TPASS, "It returned zero so it must have worked!");
615        else
616            tst_res(TFAIL | TERRNO, "statx can not stat a basic file");
617    }
618
619    static struct tst_test test = {
620        .setup = setup,
621        .cleanup = cleanup,
622        .test_all = run,
623        .tcnt = 2,
624        .min_kver = "4.11",
625        .needs_tmpdir = 1
626    };
627
628Because we are now opening a file, we need a ``cleanup`` function to close the
629file descriptors. We have to manually close the files to ensure the temporary
630directory is deleted by the test harness (see :doc:`writing_tests` for details).
631
632As a matter of good practice, the file descriptors are closed in reverse
633order. In some circumstances the order which ``cleanup`` is performed is
634significant. In those cases, resources created towards the end of ``setup`` are
635dependent to ones near the beginning. During ``cleanup`` we remove the
636dependants before their dependencies.
637
638If, for some reason, the file descriptor ``lfd`` became invalid during the test,
639but ``fd`` was still open, we do not want ``SAFE_CLOSE(lfd)`` to cause the
640``cleanup`` function to exit prematurely. If it did, then ``fd`` would remain
641open which would cause problems on some file systems.
642
643Nor do we want to call ``cleanup`` recursively. So during ``cleanup``
644``tst_brk``, and consequently the ``SAFE`` functions, do not cause the test to
645exit with ``TBROK``. Instead they just print an error message with ``TWARN``.
646
647It is not entirely necessary to check if the file descriptors have a none zero
648value before attempting to close them. However it avoids a bunch of spurious
649warning messages if we fail to open ``file_to_stat``. Test case failures can be
650difficult to interpret at the best of times, so avoid filling the log with
651noise.
652
653Check ``statx`` returns the correct number of hard links
654~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
655
656The field ``statx.stx_nlink`` should be equal to 2, right?
657
658Git-branch
659~~~~~~~~~~
660
661We are about to make some organizational changes to the test, so now would be
662a good time to branch. Then we can switch between the old and new versions, to
663check the behavior has not been changed by accident.
664
665Split the test
666--------------
667
668In our current test, we have essentially rolled two different test cases into
669one. Firstly we check if an error is returned when bad arguments are provided
670and secondly we check what happens when we stat an actual file. Quite often it
671makes sense to call ``tst_res`` multiple times in a single test case because we
672are checking different properties of the same result, but here we are clearly
673testing two different scenarios.
674
675So we should split the test in two. One obvious way to do this is to create
676``statx02.c``, but that seems like overkill in order to separate two simple test
677cases. So, for now at least, we are going to do it a different way.
678
679.. code-block:: c
680
681    ...
682
683    static void run_stat_null(void)
684    {
685        struct statx statxbuf = { 0 };
686
687        TEST(sys_statx(0, NULL, 0, 0, &statxbuf));
688        if (TST_RET == 0)
689            tst_res(TFAIL, "statx thinks it can stat NULL");
690        else if (TST_ERR == EFAULT)
691            tst_res(TPASS, "statx set errno to EFAULT as expected");
692        else
693            tst_res(TFAIL | TERRNO, "statx set errno to some unexpected value");
694    }
695
696    static void run_stat_symlink(void)
697    {
698        struct statx statxbuf = { 0 };
699
700        TEST(sys_statx(AT_FDCWD, LNAME, 0, STATX_BASIC_STATS, &statxbuf));
701        if (TST_RET == 0)
702            tst_res(TPASS, "It returned zero so it must have worked!");
703        else
704            tst_res(TFAIL | TERRNO, "statx can not stat a basic file");
705    }
706
707    static void run(unsigned int i)
708    {
709        switch(i) {
710        case 0: run_stat_null();
711        case 1: run_stat_symlink();
712        }
713    }
714
715    static struct tst_test test = {
716        .setup = setup,
717        .cleanup = cleanup,
718        .test = run,
719        .tcnt = 2,
720        .min_kver = "4.11",
721        .needs_tmpdir = 1
722    };
723
724So we have used an alternative form of the ``test`` or ``run`` callback which
725accepts an index. Some tests use this index with an array of parameters and
726expected return values. Others do something similar to the above. The index
727can be used how you want so long as each iteration calls ``tst_res`` in a
728meaningful way.
729
730If an iteration fails to return a result (i.e. call ``tst_res`` with a value
731other than ``TINFO``) then the test harness will report ``TBROK`` and print the
732iteration which failed. This prevents a scenario in your test from silently
733failing due to some faulty logic.
734
735What is wrong with the switch statement?
736~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
737
738Were you paying attention? Also see the output of ``make check``.
739
740Test a feature unique to statx
741~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
742
743So far we have not tested anything which is unique to ``statx``. So, for
744example, you could check stx_btime is correct (possibly only to within a
745margin of error) and that it differs from ``stx_mtime`` after writing to the
746file.
747
748Alternatively you could check that ``stx_dev_major`` and ``stx_dev_minor`` are set
749correctly. Note that the LTP has helper functions for creating devices and
750file systems.
751
752This could be quite a challenging exercise. You may wish to tackle an
753altogether different test scenario instead. If you get stuck just move onto
754the next section and come back later.
755
756Submitting the test for review
757------------------------------
758
759Ignoring the fact we should probably create :master:`lapi/stat.h` along with a bunch
760of fallback logic in the build system. We can now get our test ready for
761submission.
762
763The first thing you need to do before considering submitting your test is run
764``make check-statx01`` or ``make check`` in the test's directory. Again, we use
765the kernel style guidelines where possible. Next you should create a new
766branch, this will allow you to reshape your commit history without fear.
767
768After that we have the pleasure of doing an interactive ``rebase`` to clean up
769our commit history. In its current form the test only really needs a single
770commit, but if you have been using Git correctly then you should have
771many. The main reason we want to compress it to a single commit, is to make
772the LTP's Git-log readable. It also allows us to write a coherent description
773of the work as a whole in retrospective. Although, when adding a new test, the
774test description in the code will probably make the commit message redundant.
775
776Anyway, as an example, we shall look at my personal commit history from this
777tutorial and ``rebase`` it. You should try following along with your own
778repository. First lets look at the commit history since we branched from
779master.
780
781.. code-block:: bash
782
783    git log --oneline master..HEAD
784    152d39fe7 (HEAD -> tutorial-rebase2, tutorial-rebase) tutorial: Start Submitting patch section
785    70f7ce7ce statx01: Stop checkpatch from complaining
786    bb0332bd7 tutorial: Fix review problems
787    6a87a084a statx01: Fix review problems
788    d784b1e85 test-writing-guidelines: Remove old API argument
789    c26e1be7a fixup! tutorial
790    1e24a5fb5 (me/tutorial-rebase) fixup! tutorial
791    568a3f7be fixup! tutorial
792    09dd2c829 statx: stage 6
793    bfeef7902 statx: stage 5b
794    76e03d714 statx: stage 5a
795    98f5bc7ac statx: stage 4
796    6f8c16438 statx: stage 3 (Add statx01)
797    5d93b84d8 Add statx and other syscall numbers
798    5ca627b78 tutorial: Add a step-by-step C test tutorial
799
800So we have told git to show all the commits which don't exist in ``master``, but
801are in ``HEAD``, where ``HEAD`` is the top of the current branch. The current
802branch is ``tutorial-rebase2`` which I just created. I have already done one
803``rebase`` and submitted a patch for review, so my original branch was just called
804``tutorial``.
805
806As usual my commit history is starting to look like a bit of mess! There is
807even a commit in there which should not be in the this branch (Remove old API
808argument), however it can be ignored for now and 'cherry picked' into a new branch
809later.
810
811For my patch I actually need at least two commits, one which contains the
812tutorial text and one which contains the test and associated files. So first
813of all I want to 'squash' (amalgamate) all the commits appended with
814``tutorial:`` into the bottom commit.
815
816.. code-block:: bash
817
818    $ git rebase -i 5ca627b78\^
819    ...
820
821This begins an interactive ``rebase`` where commit ``5ca6427b78`` is the earliest
822commit we want to edit. The ``^`` symbol after the commit hash, specifies the
823commit before this one. The interactive ``rebase`` command takes the last commit
824we want to keep unaltered as it's argument (in other words it takes a
825non-inclusive range).
826
827Upon entering a similar command you will be presented with a text file
828similar to the following. The file should be displayed in your text editor of
829choice, if it doesn't, then you may change the editor variable in
830``.gitconfig``.
831
832.. code-block:: bash
833
834    pick 5ca627b78 tutorial: Add a step-by-step C test tutorial
835    pick 5d93b84d8 Add statx and other syscall numbers
836    pick 6f8c16438 statx: stage 3 (Add statx01)
837    pick 98f5bc7ac statx: stage 4
838    pick 76e03d714 statx: stage 5a
839    pick bfeef7902 statx: stage 5b
840    pick 09dd2c829 statx: stage 6
841    pick 568a3f7be fixup! tutorial
842    pick 1e24a5fb5 fixup! tutorial
843    pick c26e1be7a fixup! tutorial
844    pick d784b1e85 test-writing-guidelines: Remove old API argument
845    pick 6a87a084a statx01: Fix review problems
846    pick bb0332bd7 tutorial: Fix review problems
847    pick 70f7ce7ce statx01: Stop checkpatch from complaining
848    pick 152d39fe7 tutorial: Start Submitting patch section
849
850The last commit from Git-log is shown at the top. The left hand column
851contains the commands we want to run on each commit. ``pick`` just means we
852re-apply the commit as-is. We can reorder the lines to apply the commits in a
853different order, but we need to be careful when reordering commits to the same
854file. If your ``rebase`` results in a merge conflict, then you have probably
855reordered some commits which contained changes to the same piece of code.
856
857Perhaps a better name for the interactive ``rebase`` command would be 'replay'. As
858we pick a point in the commit history, undo all those commits before that
859point, then reapply them one at a time. During the replay we can reorder the
860commits, drop, merge, split and edit them, creating a new history.
861
862The commands I am going to use are ``reword`` and ``fixup``. The ``reword`` command
863allows you to edit a single commit's message. The 'fixup' command 'squashes' a
864commit into the commit above/preceding it, merging the two commits into
865one. The commit which has ``fixup`` applied has its commit message deleted. If
866you think a commit might have something useful in its message then you can use
867``squash`` instead.
868
869.. code-block:: bash
870
871    reword 5ca627b78 tutorial: Add a step-by-step C test tutorial
872    fixup 568a3f7be fixup! tutorial
873    fixup 1e24a5fb5 fixup! tutorial
874    fixup c26e1be7a fixup! tutorial
875    fixup bb0332bd7 tutorial: Fix review problems
876    fixup 152d39fe7 tutorial: Start Submitting patch section
877    fixup 276edecab tutorial: Save changes before rebase
878    pick 5d93b84d8 Add statx and other syscall numbers
879    pick 6f8c16438 statx: stage 3 (Add statx01)
880    pick 98f5bc7ac statx: stage 4
881    pick 76e03d714 statx: stage 5a
882    pick bfeef7902 statx: stage 5b
883    pick 09dd2c829 statx: stage 6
884    pick d784b1e85 test-writing-guidelines: Remove old API argument
885    pick 6a87a084a statx01: Fix review problems
886
887So all the commits marked with ``fixup`` will be re-played by Git immediately
888after 5ca62 at the top. A new commit will then be created with the amalgamated
889changes of all the commits and 5ca62's log message. It turns out that I didn't
890need to reword anything, but there is no harm in checking. It is easy to
891forget the ``Signed-off-by:`` line.
892
893I could now do the same for the commits to the ``statx`` test, making the commit
894message prefixes consistent. However I am not actually going to submit the
895test (yet).
896
897I won't attempt to show you this, but if you need to do the opposite and split
898apart a commit. It is also possible using Git-rebase by marking a line with
899``edit``. This will pause Git just after replaying the marked commit. You can
900then use a 'soft' Git-reset to bring the selected commit's changes back into
901the 'index' where you are then able to un-stage some parts before
902re-committing.
903
904You can also use ``edit`` and ``git commit --amend`` together to change a commit
905deep in your history, but without resetting the 'index'. The 'index' contains
906changes which you have staged with :git_man:`add`, but not yet committed.
907
908So now that the commit history has been cleaned up, we need to submit a patch
909to the mailing list or make a pull request on GitHub. The mailing list is the
910preferred place to make submissions and is more difficult for most people, so
911I will only cover that method.
912
913Just before we create the patch, we need to check that our changes will still
914apply to the master branch without problems. To do this we can use another
915type of ``rebase`` and then try rebuilding and running the test.
916
917.. code-block:: bash
918
919    git checkout master
920    git pull origin
921    git checkout tutorial-rebase2
922    git rebase master
923
924Above, I update the master branch and then replay our changes onto it using
925``git rebase master``. You may find that after the rebase there is a merge
926conflict. This will result in something which looks like the following (taken
927from a Makefile conflict which was caused by reordering commits in a ``rebase``).
928
929.. code-block:: diff
930
931    <<<<<<< HEAD
932    cve-2016-7117:	LDFLAGS += -lpthread
933    =======
934    cve-2014-0196:	LDFLAGS += -lpthread -lutil -lrt
935    cve-2016-7117:	LDFLAGS += -lpthread -lrt
936    >>>>>>> 4dbfb8e79... Add -lrt
937
938The first line tells us this is the beginning of a conflict. The third line
939separates the two conflicting pieces of content and the last line is the end
940of the conflict. Usually, all you need to do is remove the lines you don't
941want, stage the changes and continue the ``rebase`` with ``git rebase
942--continue``.
943
944In order to create a patch e-mail we use :git_man:`format-patch`,
945we can then send that e-mail using :git_man:`send-email`.
946It is also possible to import the patch (``mbox``) file into a number of e-mail
947programs.
948
949.. code-block:: bash
950
951    $ git format-patch -1 -v 2 -o output --to [email protected] fd3cc8596
952    output/v2-0001-tutorial-Add-a-step-by-step-C-test-tutorial.patch
953
954The first argument ``-1`` specifies we want one commit from fd3cc8596
955onwards. If we wanted this commit and the one after it we could specify ``-2``
956instead.
957
958This is my second patch submission so I have used ``-v 2``, which indicates this
959is the second version of a patch set. The ``-o`` option specifies the output
960directory (literally called ``output``). The ``--to`` option adds the ``To:`` e-mail
961header, which I have set to the LTP mailing list.
962
963We can then send this patch with the following command sans ``--dry-run``.
964
965.. code-block:: bash
966
967    git send-email --dry-run output/v2-0001-tutorial-Add-a-step-by-step-C-test-tutorial.patch
968
969Git will ask some questions (which you can ignore) and then tell you what it
970would do if this weren't a dry-run. In order for this to work you have to have
971a valid SMTP server set in ``.gitconfig`` and also be signed up to the LTP
972mailing list under the same e-mail address you have configured in Git. You can
973sign up at https://lists.linux.it/listinfo/ltp.
974
975Doing code review
976-----------------
977
978While waiting for your test to be reviewed, you are invited and encouraged to
979review other contributors' code. This may seem bizarre when you are completely
980new to the project, but there are two important ways in which you can
981contribute here:
982
983A.   Point out logical errors in the code.
984B.   Improve your own understanding
985
986It doesn't matter whether you know the canonical way of writing an LTP test in
987C. An error of logic, when properly explained, is usually indisputable. These
988are the most important errors to find as they always result in false test
989results. Once someone points out such an error it is usually obvious to
990everyone that it is a bug and needs to be fixed.
991
992Obviously testing the patch is one way of finding errors. You can apply patches
993using :git_man:`am`. Then it is just a case of compiling and running the tests.
994
995Finally, reading and attempting to comment on other peoples patches, gives
996you a better understanding of the reviewers perspective. This is better for
997the project and for you.
998
999Style and organizational issues are best left to after you have found logical
1000errors.
1001
1002Final notes
1003-----------
1004
1005Hopefully you can now grasp the structure of an LTP test and have some idea of
1006what is available in the LTP test library. There are a vast number of library
1007functions available (mainly located in include and lib), some of which are
1008documented in the test writing guidelines and many of which are not.
1009
1010We have only scratched the surface of the immense technical complexity of
1011systems programming across multiple Kernel and C lib versions as well as
1012different hardware architectures. The important thing to take away from this
1013is that you have to be conscientious of what will happen on systems different
1014from yours. The LTP has a huge and varied user base, so situations you may
1015think are unlikely can and do happen to somebody.
1016
1017Of course you don't want to spend time allowing for situations which may never
1018arise either, so you have to do your research and think about each situation
1019critically. The more systems you can test on before submitting your changes,
1020the better, although we understand not everyone has access to a lab.
1021
1022One important topic which has not been covered by this tutorial, is
1023multi-process or multi-threaded testing. The LTP library functions work inside
1024child processes and threads, but their semantics change slightly. There are
1025also various helper functions for synchronizing and forking processes.
1026
1027.. note::
1028
1029    When it comes time to submit a test, the preferred way to do it is on the
1030    mailing list although you can also use GitHub. The LTP follows similar rules
1031    to the kernel for formatting and submitting patches. Generally speaking the
1032    review cycle is easier for small patches, so try to make small changes or
1033    additions where possible.
1034