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