1LTP Style Guide 2=============== 3 4********************************************************************** 5Welcome to the *LTP Project Coding Guideline* document! This document is 6designed to guide committers and contributors in producing consistent, quality 7deterministic testcases and port testcases from other sources to LTP so that 8they can be easily maintained by project members and external contributors. 9 10Please refer to the *Linux Kernel Coding Guidelines* unless stated otherwise 11in this document. 12 13Changelog: 14 15 * Initial version: Ngie Cooper <yaneurabeya@gmail.com> 16 * Reformatted for asciidoc: Cyril Hrubis <chrubis@suse.cz> 17********************************************************************** 18 19Using libltp 20------------ 21 22Of course the manpages should be the primary source of information in 23terms of how you should use libltp, but this section provides a quick 24set of guidelines to hit the ground running with libltp. 25 26When you can use libltp please observe the following guidelines: 27 281. Should I use tst_*() interface in forked processes? 29~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 30 31No, only use libltp in non-forked processes and functions +perror(3)+ / 32+exit(3)+ otherwise. Reason being: 33 34 * Calling +tst_resm()+ from multiple processes is messy. 35 * Calling cleanup can break test execution. 36 * Establishing a complicated scheme of tracking the testcase state 37 for teardown is undesirable. 38 392. Use SAFE_* macros 40~~~~~~~~~~~~~~~~~~~~ 41 42Use +SAFE_*+ macros (see +safe_macros.h+) instead of bare calls to libcalls and 43syscalls. This will: 44 45 * Reduce number of lines wasted on repeated in multiple files with 46 error catching logic. 47 * Ensure that error catching is consistent and informative; all 48 +SAFE_*+ macros contain the line number of the failure as well as 49 the file where the failure occurred by design. So unless the 50 stack gets stomped on, you will be able to determine where 51 the failures occurred if and when they do occur with absolute 52 determinism. 53 * Mute some unchecked return code warnings. 54 553. Don't call cleanup() from within setup() 56~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 57 58Unless you need to clean up or reset system state that wouldn't otherwise be 59handled by a proper release of all OS resources. 60 61This includes (but is not limited to): 62 63 * Memory mapped pages. 64 * Mounted filesystems. 65 * File descriptors (if they're in temporary directories, etc). 66 * Temporary files / directories. 67 * Waiting for child processes. 68 * +/proc+ & +/sysfs+ tunable states. 69 70You don't need to clean up the following: 71 72 * +malloc(3)+'ed memory. 73 * Read-only file descriptors in persistent paths (i.e. not 74 temporary directories). 75 76Please be aware of some of the caveats surrounding forking, 77exec'ing and descriptor inheritance, etc. 78 794. Call APIs that don't require freeing up resources on failure first 80~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 81 82 * +tst_require_root()+ 83 * +tst_sig(...)+ 84 * +malloc(3)+ 85 * +tst_tmpdir()+ 86 87That way you can simplify your setup and avoid calling cleanup whenever 88possible! 89 905. If the test needs to run as root 91~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 92 93If the test need to run as root, check to make sure that you're root 94*before doing any other setup* via +tst_require_root()+. 95 966. No custom reporting APIs 97~~~~~~~~~~~~~~~~~~~~~~~~~~~ 98 99Don't roll your own reporting APIs unless you're porting testcases or are 100concerned about portability. 101 1027. Handle TBROK and TFAIL correctly 103~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 104 105Use +TBROK+ when an unexpected failure unrelated to the goal of the testcase 106occurred, and use +TFAIL+ when an unexpected failure related to the goal of 107the testcase occurred. 108 1098. Don't roll your own syscall numbers 110~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 111 112Header +lapi/syscalls.h+ exists for this purpose and does a pretty 113dang good job. 114 1159. Keep errors as short and sweet as possible 116~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 117 118For example: 119[source,c] 120---------------------------------------------------- 121if (fork() == -1) 122 tst_brkm(TBROK, cleanup, "fork failed"); 123 124/* or */ 125 126if (fork() == -1) 127 tst_brkm(TBROK, cleanup, "fork # 1 failed"); 128 129if (fork() == -1) 130 tst_brkm(TBROK, cleanup, "fork # 2 failed"); 131---------------------------------------------------- 132 133If you can't determine where the failure has occurred in a testcase based on 134the entire content of the failure messages, then determining the cause of 135failure may be impossible. 136 13710. Reporting errno and the TEST() macro 138~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 139 140Don't roll your own +errno+ / +strerror()+ printout when you use +tst_resm()+ 141calls. Use either +TERRNO+ or +TTERRNO+. Similarly, if a testcase passed and 142it's obvious why it passed (for example a function call returns +0+ or 143+TEST_RETURN == 0+). 144 145[source,c] 146------------------------------------------------------------------------------- 147/* Example without TEST(...) macro */ 148 149exp_errno = ENOENT; 150 151if (fn() == -1) { 152 /* 153 * Using TERRNO here is valid because the error case 154 * isn't static. 155 */ 156 if (exp_errno == ENOENT) 157 tst_resm(TPASS|TERRNO, 158 "fn failed as expected"); 159 /* 160 * Using strerror(TEST_ERRNO) here is valid because 161 * the error case isn't static. 162 */ 163 else 164 tst_resm(TFAIL|TERRNO, 165 "fn failed unexpectedly; expected " 166 "%d - %s", 167 exp_errno, strerror(exp_errno)); 168} else 169 tst_resm(TBROK, "fn passed unexpectedly"); 170 171/* Example using TEST(...) macro */ 172 173TEST(fn()); 174if (TEST_RETURN == 0) 175 tst_resm(TPASS, "fn passed as expected"); 176else 177 tst_resm(TFAIL|TTERRNO, "fn failed"); 178------------------------------------------------------------------------------- 179 180[NOTE] 181The +TEST()+ macro is not thread-safe as it saves return value and errno into 182global variable. 183 18412. Use tst_brkm() when possible 185~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 186 187Use... 188[source,c] 189------------------------------ 190tst_brkm(TBROK, cleanup, ...); 191------------------------------ 192...instead of... 193[source,c] 194------------------------------ 195tst_resm(TBROK, ...); 196cleanup(); 197tst_exit(); 198------------------------------ 199 200[NOTE] 201As you see the +tst_brkm()+ no longer requires non +NULL+ cleanup_fn argument 202in order to call +tst_exit()+. 203 20413. Indentation 205~~~~~~~~~~~~~~~ 206 207Use hard tabs for first-level indentation, and 4 spaces for every line longer 208than 80 columns. Use a linebreak with string constants in format functions 209like +*printf()+, the +tst_resm()+ APIs, etc. 210 211Example: 212[source,c] 213------------------------------------------------------------------------------- 214if ((this_is_a_poorly_formed_really_long_variable = function_call()) == NULL && 215 statement1() && i < j && l != 5) { 216 /* Use tabs here */ 217 printf("The rain in Spain falls mainly in the plain.\nThe quick brown " 218 "fox jumped over the lazy yellow dog\n"); 219 tst_resm(TPASS, 220 "Half would turn and fight. The other half would try to swim " 221 "across. But my uncle told me about a few that... they'd swim " 222 "halfway out, turn with the current, and ride it all the way out " 223 "to sea. Fisherman would find them a mile offshore, swimming."); 224} 225------------------------------------------------------------------------------- 226 22714. System headers 228~~~~~~~~~~~~~~~~~~ 229 230Don't use +linux/+ headers if at all possible. Usually they are replaced with 231+sys/+ headers as things work their way into glibc. Furthermore, +linux/+ 232headers get shuffled around a lot more than their +sys/+ counterparts it 233seems. 234 23515. Signal handlers 236~~~~~~~~~~~~~~~~~~~~ 237 238Avoid doing tricky things in signal handlers. Calling most of the libc 239functions from signal handler may lead to deadlocks or memory corruption. If 240you really need to do anything more complicated that +should_run = 0;+ in your 241signal handler consult +man 7 signal+ for async-signal-safe functions. 242 243Porting Testcases 244----------------- 245 246If one of the following is true... 247 248 1. You are porting a testcase directly to LTP which doesn't need to 249 be ported outside of Linux. 250 2. The beforementioned project or source is no longer contributing 251 changes to the testcases. 252 253Then please fully port the testcase(s) to LTP. Otherwise, add robust porting 254shims around the testcases using libltp APIs to reduce longterm maintenance 255and leave the sources alone so it's easier to sync the testcases from the 256upstream source. 257 258New Testcases 259------------- 260 261 1. Always use libltp for Linux centric tests. No ifs, ands, or buts 262 about it. 263 2. Sort headers, like: 264 265[source,c] 266--------------------------------------------------------------------------- 267/* 268 * sys/types.h is usually (but not always) required by POSIX 269 * APIs. 270 */ 271#include <sys/types.h> 272/* sys headers, alphabetical order. */ 273/* directory prefixed libc headers. */ 274/* non-directory prefixed libc headers. */ 275/* directory prefixed non-libc (third-party) headers. */ 276/* non-directory prefixed non-libc (third-party) headers. */ 277/* Sourcebase relative includes. */ 278 279/* e.g. */ 280 281#include <sys/types.h> 282#include <sys/stat.h> 283#include <netinet/icmp.h> 284#include <stdio.h> 285#include <dbus-1.0/dbus/dbus.h> 286#include <archive.h> 287#include "foo/bar.h" 288#include "test.h" 289--------------------------------------------------------------------------- 290 2913. Comments 292~~~~~~~~~~~ 293 294Do not use obvious comments ("child process", "parse options", etc). This 295leads to visual comment noise pollution. 296 2974. Inline assignments 298~~~~~~~~~~~~~~~~~~~~ 299 300Don't do inline assignment, i.e. 301 302[source,c] 303--------------------------------------------------------------------------- 304int foo = 0; 305 306Use separate statements: 307 308int foo; 309 310foo = 0; 311--------------------------------------------------------------------------- 312 313The only exception to this is when you define global variables. 314 3155. How to assert test is running as root? 316~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 317 318Your testcase should be runnable as root and non-root. What does that mean? It 319should fail gracefully as non-root if it has insufficient privileges, or use 320+tst_require_root()+ if root access is absolutely required. 321 322A lot of newer testcases don't honor this fact and it causes random 323unnecessary errors when run as non-privileged users. 324 3256. Do I need to create temporary directory? 326~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 327 328Use +tst_tmpdir()+ if your testcase will: 329 330* Create temporary files. 331* Dump core. 332* Etc. Otherwise, don't bother with the API. 333 334[NOTE] 335If you created temporary directory with +tst_tmpdir()+ don't forget to call 336+tst_rmdir()+ when the test is cleaning up. This is *NOT* done automatically. 337 3387. Setting up signal handlers 339~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 340 341Use +tst_sig()+ instead of bothering with sigaction / signal. This reduces 342potential of leaving a mess around if your test doesn't exit cleanly (now, 343there are some signals that can't be blocked, i.e. +SIGKILL+ and +SIGSTOP+, 344but the rest can be caught). 345 3468. Basic template 347~~~~~~~~~~~~~~~~~ 348 349The following is a basic testcase template: 350[source,c] 351--------------------------------------------------------------------------- 352#include "test.h" 353 354char *TCID = "testname"; 355int TST_TOTAL = /* ... */ 356 357static void setup(void) 358{ 359 /* ... */ 360 361 tst_require_root(); 362 363 tst_tmpdir(); 364 365 /* ... */ 366 367 TEST_PAUSE; 368} 369 370static void cleanup(void) 371{ 372 373 TEST_CLEANUP; 374 375 /* ... */ 376 377 tst_rmdir(); 378} 379 380int main(void) 381{ 382 /* ... */ 383 384 setup(); /* Optional */ 385 386 cleanup(); /* Optional */ 387 tst_exit(); /* DON'T FORGET THIS -- this must be last! */ 388} 389--------------------------------------------------------------------------- 390 391Fixing Legacy Testcases 392----------------------- 393 394Unless you interested in exercising self-masochism, do minimal changes 395to testcases when fixing them so it's easier to track potential 396breakage in testcases after a commit is made (mistakes happen and no 397one is perfect). If it works after you fix it -- great -- move on. 398It's more the job of the committers / maintainers to do things like 399style commits. 400 401It's better to focus on adding more content to LTP as a contributor 402and fixing functional issues with existing tests than it is worrying 403about whitespace, unnecessary pedanticness of function calls, etc. 404 405As ugly as the style is in the surrounding code may be, stick to it. 406Otherwise anyone reading the code will cringe at the chaos and 407non-uniform `style', and it will be more difficult to fix later. 408 409 410Fixing Open Posix Testsuite 411--------------------------- 412 413The *Open Posix Testsuite* does not use libltp interface and moreover aims to 414be usable on any *POSIX* compatible operating system. 415 416So *NO* Linux, BSD specific syscalls there. 417 418Contribution to the project 419--------------------------- 420 421Since CVS is effectively dead for LTP proper, we ask that you submit 422patches that are git friendly and patchable. 423 424Guidelines for submitting patches are as follows: 425 4261. Use +git commit -s+ . You know you want to ;) .. (you may need to 427 submit a correct signed-off-by line, e.g. use git config first). 4282. Provide a short (<= 50 character) description of the commit. 4293. Provide a little more terse (1-2 paragraph maximum, <= 72 character 430 lines) description of what the commit does. 4314. Format the email with +git format-patch --attach+ command . 432 433Example of a commit message: 434 435------------------------------------------------------------------- 436Short description of my commit. 437 438This is a much longer description of my commit. Blah blah blah blah 439blah blah blah blah blah. 440 441Signed-off-by: John Smith <dev@null> 442------------------------------------------------------------------- 443