You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
442 lines
13 KiB
442 lines
13 KiB
LTP Style Guide |
|
=============== |
|
|
|
********************************************************************** |
|
Welcome to the *LTP Project Coding Guideline* document! This document is |
|
designed to guide committers and contributors in producing consistent, quality |
|
deterministic testcases and port testcases from other sources to LTP so that |
|
they can be easily maintained by project members and external contributors. |
|
|
|
Please refer to the *Linux Kernel Coding Guidelines* unless stated otherwise |
|
in this document. |
|
|
|
Changelog: |
|
|
|
* Initial version: Ngie Cooper <yaneurabeya@gmail.com> |
|
* Reformatted for asciidoc: Cyril Hrubis <chrubis@suse.cz> |
|
********************************************************************** |
|
|
|
Using libltp |
|
------------ |
|
|
|
Of course the manpages should be the primary source of information in |
|
terms of how you should use libltp, but this section provides a quick |
|
set of guidelines to hit the ground running with libltp. |
|
|
|
When you can use libltp please observe the following guidelines: |
|
|
|
1. Should I use tst_*() interface in forked processes? |
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
|
|
|
No, only use libltp in non-forked processes and functions +perror(3)+ / |
|
+exit(3)+ otherwise. Reason being: |
|
|
|
* Calling +tst_resm()+ from multiple processes is messy. |
|
* Calling cleanup can break test execution. |
|
* Establishing a complicated scheme of tracking the testcase state |
|
for teardown is undesirable. |
|
|
|
2. Use SAFE_* macros |
|
~~~~~~~~~~~~~~~~~~~~ |
|
|
|
Use +SAFE_*+ macros (see +safe_macros.h+) instead of bare calls to libcalls and |
|
syscalls. This will: |
|
|
|
* Reduce number of lines wasted on repeated in multiple files with |
|
error catching logic. |
|
* Ensure that error catching is consistent and informative; all |
|
+SAFE_*+ macros contain the line number of the failure as well as |
|
the file where the failure occurred by design. So unless the |
|
stack gets stomped on, you will be able to determine where |
|
the failures occurred if and when they do occur with absolute |
|
determinism. |
|
* Mute some unchecked return code warnings. |
|
|
|
3. Don't call cleanup() from within setup() |
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
|
|
|
Unless you need to clean up or reset system state that wouldn't otherwise be |
|
handled by a proper release of all OS resources. |
|
|
|
This includes (but is not limited to): |
|
|
|
* Memory mapped pages. |
|
* Mounted filesystems. |
|
* File descriptors (if they're in temporary directories, etc). |
|
* Temporary files / directories. |
|
* Waiting for child processes. |
|
* +/proc+ & +/sysfs+ tunable states. |
|
|
|
You don't need to clean up the following: |
|
|
|
* +malloc(3)+'ed memory. |
|
* Read-only file descriptors in persistent paths (i.e. not |
|
temporary directories). |
|
|
|
Please be aware of some of the caveats surrounding forking, |
|
exec'ing and descriptor inheritance, etc. |
|
|
|
4. Call APIs that don't require freeing up resources on failure first |
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
|
|
|
* +tst_require_root()+ |
|
* +tst_sig(...)+ |
|
* +malloc(3)+ |
|
* +tst_tmpdir()+ |
|
|
|
That way you can simplify your setup and avoid calling cleanup whenever |
|
possible! |
|
|
|
5. If the test needs to run as root |
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
|
|
|
If the test need to run as root, check to make sure that you're root |
|
*before doing any other setup* via +tst_require_root()+. |
|
|
|
6. No custom reporting APIs |
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
|
|
|
Don't roll your own reporting APIs unless you're porting testcases or are |
|
concerned about portability. |
|
|
|
7. Handle TBROK and TFAIL correctly |
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
|
|
|
Use +TBROK+ when an unexpected failure unrelated to the goal of the testcase |
|
occurred, and use +TFAIL+ when an unexpected failure related to the goal of |
|
the testcase occurred. |
|
|
|
8. Don't roll your own syscall numbers |
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
|
|
|
Header +linux_syscall_numbers.h+ exists for this purpose and does a pretty |
|
dang good job. |
|
|
|
9. Keep errors as short and sweet as possible |
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
|
|
|
For example: |
|
[source,c] |
|
---------------------------------------------------- |
|
if (fork() == -1) |
|
tst_brkm(TBROK, cleanup, "fork failed"); |
|
|
|
/* or */ |
|
|
|
if (fork() == -1) |
|
tst_brkm(TBROK, cleanup, "fork # 1 failed"); |
|
|
|
if (fork() == -1) |
|
tst_brkm(TBROK, cleanup, "fork # 2 failed"); |
|
---------------------------------------------------- |
|
|
|
If you can't determine where the failure has occurred in a testcase based on |
|
the entire content of the failure messages, then determining the cause of |
|
failure may be impossible. |
|
|
|
10. Reporting errno and the TEST() macro |
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
|
|
|
Don't roll your own +errno+ / +strerror()+ printout when you use +tst_resm()+ |
|
calls. Use either +TERRNO+ or +TTERRNO+. Similarly, if a testcase passed and |
|
it's obvious why it passed (for example a function call returns +0+ or |
|
+TEST_RETURN == 0+). |
|
|
|
[source,c] |
|
------------------------------------------------------------------------------- |
|
/* Example without TEST(...) macro */ |
|
|
|
exp_errno = ENOENT; |
|
|
|
if (fn() == -1) { |
|
/* |
|
* Using TERRNO here is valid because the error case |
|
* isn't static. |
|
*/ |
|
if (exp_errno == ENOENT) |
|
tst_resm(TPASS|TERRNO, |
|
"fn failed as expected"); |
|
/* |
|
* Using strerror(TEST_ERRNO) here is valid because |
|
* the error case isn't static. |
|
*/ |
|
else |
|
tst_resm(TFAIL|TERRNO, |
|
"fn failed unexpectedly; expected " |
|
"%d - %s", |
|
exp_errno, strerror(exp_errno)); |
|
} else |
|
tst_resm(TBROK, "fn passed unexpectedly"); |
|
|
|
/* Example using TEST(...) macro */ |
|
|
|
TEST(fn()); |
|
if (TEST_RETURN == 0) |
|
tst_resm(TPASS, "fn passed as expected"); |
|
else |
|
tst_resm(TFAIL|TTERRNO, "fn failed"); |
|
------------------------------------------------------------------------------- |
|
|
|
[NOTE] |
|
The +TEST()+ macro is not thread-safe as it saves return value and errno into |
|
global variable. |
|
|
|
12. Use tst_brkm() when possible |
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
|
|
|
Use... |
|
[source,c] |
|
------------------------------ |
|
tst_brkm(TBROK, cleanup, ...); |
|
------------------------------ |
|
...instead of... |
|
[source,c] |
|
------------------------------ |
|
tst_resm(TBROK, ...); |
|
cleanup(); |
|
tst_exit(); |
|
------------------------------ |
|
|
|
[NOTE] |
|
As you see the +tst_brkm()+ no longer requires non +NULL+ cleanup_fn argument |
|
in order to call +tst_exit()+. |
|
|
|
13. Indentation |
|
~~~~~~~~~~~~~~~ |
|
|
|
Use hard tabs for first-level indentation, and 4 spaces for every line longer |
|
than 80 columns. Use a linebreak with string constants in format functions |
|
like +*printf()+, the +tst_resm()+ APIs, etc. |
|
|
|
Example: |
|
[source,c] |
|
------------------------------------------------------------------------------- |
|
if ((this_is_a_poorly_formed_really_long_variable = function_call()) == NULL && |
|
statement1() && i < j && l != 5) { |
|
/* Use tabs here */ |
|
printf("The rain in Spain falls mainly in the plain.\nThe quick brown " |
|
"fox jumped over the lazy yellow dog\n"); |
|
tst_resm(TPASS, |
|
"Half would turn and fight. The other half would try to swim " |
|
"across. But my uncle told me about a few that... they'd swim " |
|
"halfway out, turn with the current, and ride it all the way out " |
|
"to sea. Fisherman would find them a mile offshore, swimming."); |
|
} |
|
------------------------------------------------------------------------------- |
|
|
|
14. System headers |
|
~~~~~~~~~~~~~~~~~~ |
|
|
|
Don't use +linux/+ headers if at all possible. Usually they are replaced with |
|
+sys/+ headers as things work their way into glibc. Furthermore, +linux/+ |
|
headers get shuffled around a lot more than their +sys/+ counterparts it |
|
seems. |
|
|
|
15. Signal handlers |
|
~~~~~~~~~~~~~~~~~~~~ |
|
|
|
Avoid doing tricky things in signal handlers. Calling most of the libc |
|
functions from signal handler may lead to deadlocks or memory corruption. If |
|
you really need to do anything more complicated that +should_run = 0;+ in your |
|
signal handler consult +man 7 signal+ for async-signal-safe functions. |
|
|
|
Porting Testcases |
|
----------------- |
|
|
|
If one of the following is true... |
|
|
|
1. You are porting a testcase directly to LTP which doesn't need to |
|
be ported outside of Linux. |
|
2. The beforementioned project or source is no longer contributing |
|
changes to the testcases. |
|
|
|
Then please fully port the testcase(s) to LTP. Otherwise, add robust porting |
|
shims around the testcases using libltp APIs to reduce longterm maintenance |
|
and leave the sources alone so it's easier to sync the testcases from the |
|
upstream source. |
|
|
|
New Testcases |
|
------------- |
|
|
|
1. Always use libltp for Linux centric tests. No ifs, ands, or buts |
|
about it. |
|
2. Sort headers, like: |
|
|
|
[source,c] |
|
--------------------------------------------------------------------------- |
|
/* |
|
* sys/types.h is usually (but not always) required by POSIX |
|
* APIs. |
|
*/ |
|
#include <sys/types.h> |
|
/* sys headers, alphabetical order. */ |
|
/* directory prefixed libc headers. */ |
|
/* non-directory prefixed libc headers. */ |
|
/* directory prefixed non-libc (third-party) headers. */ |
|
/* non-directory prefixed non-libc (third-party) headers. */ |
|
/* Sourcebase relative includes. */ |
|
|
|
/* e.g. */ |
|
|
|
#include <sys/types.h> |
|
#include <sys/stat.h> |
|
#include <netinet/icmp.h> |
|
#include <stdio.h> |
|
#include <dbus-1.0/dbus/dbus.h> |
|
#include <archive.h> |
|
#include "foo/bar.h" |
|
#include "test.h" |
|
--------------------------------------------------------------------------- |
|
|
|
3. Comments |
|
~~~~~~~~~~~ |
|
|
|
Do not use obvious comments ("child process", "parse options", etc). This |
|
leads to visual comment noise pollution. |
|
|
|
4. Inline assignments |
|
~~~~~~~~~~~~~~~~~~~~ |
|
|
|
Don't do inline assignment, i.e. |
|
|
|
[source,c] |
|
--------------------------------------------------------------------------- |
|
int foo = 0; |
|
|
|
Use separate statements: |
|
|
|
int foo; |
|
|
|
foo = 0; |
|
--------------------------------------------------------------------------- |
|
|
|
The only exception to this is when you define global variables. |
|
|
|
5. How to assert test is running as root? |
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
|
|
|
Your testcase should be runnable as root and non-root. What does that mean? It |
|
should fail gracefully as non-root if it has insufficient privileges, or use |
|
+tst_require_root()+ if root access is absolutely required. |
|
|
|
A lot of newer testcases don't honor this fact and it causes random |
|
unnecessary errors when run as non-privileged users. |
|
|
|
6. Do I need to create temporary directory? |
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
|
|
|
Use +tst_tmpdir()+ if your testcase will: |
|
|
|
* Create temporary files. |
|
* Dump core. |
|
* Etc. Otherwise, don't bother with the API. |
|
|
|
[NOTE] |
|
If you created temporary directory with +tst_tmpdir()+ don't forget to call |
|
+tst_rmdir()+ when the test is cleaning up. This is *NOT* done automatically. |
|
|
|
7. Setting up signal handlers |
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
|
|
|
Use +tst_sig()+ instead of bothering with sigaction / signal. This reduces |
|
potential of leaving a mess around if your test doesn't exit cleanly (now, |
|
there are some signals that can't be blocked, i.e. +SIGKILL+ and +SIGSTOP+, |
|
but the rest can be caught). |
|
|
|
8. Basic template |
|
~~~~~~~~~~~~~~~~~ |
|
|
|
The following is a basic testcase template: |
|
[source,c] |
|
--------------------------------------------------------------------------- |
|
#include "test.h" |
|
|
|
char *TCID = "testname"; |
|
int TST_TOTAL = /* ... */ |
|
|
|
static void setup(void) |
|
{ |
|
/* ... */ |
|
|
|
tst_require_root(); |
|
|
|
tst_tmpdir(); |
|
|
|
/* ... */ |
|
|
|
TEST_PAUSE; |
|
} |
|
|
|
static void cleanup(void) |
|
{ |
|
|
|
TEST_CLEANUP; |
|
|
|
/* ... */ |
|
|
|
tst_rmdir(); |
|
} |
|
|
|
int main(void) |
|
{ |
|
/* ... */ |
|
|
|
setup(); /* Optional */ |
|
|
|
cleanup(); /* Optional */ |
|
tst_exit(); /* DON'T FORGET THIS -- this must be last! */ |
|
} |
|
--------------------------------------------------------------------------- |
|
|
|
Fixing Legacy Testcases |
|
----------------------- |
|
|
|
Unless you interested in exercising self-masochism, do minimal changes |
|
to testcases when fixing them so it's easier to track potential |
|
breakage in testcases after a commit is made (mistakes happen and no |
|
one is perfect). If it works after you fix it -- great -- move on. |
|
It's more the job of the committers / maintainers to do things like |
|
style commits. |
|
|
|
It's better to focus on adding more content to LTP as a contributor |
|
and fixing functional issues with existing tests than it is worrying |
|
about whitespace, unnecessary pedanticness of function calls, etc. |
|
|
|
As ugly as the style is in the surrounding code may be, stick to it. |
|
Otherwise anyone reading the code will cringe at the chaos and |
|
non-uniform `style', and it will be more difficult to fix later. |
|
|
|
|
|
Fixing Open Posix Testsuite |
|
--------------------------- |
|
|
|
The *Open Posix Testsuite* does not use libltp interface and moreover aims to |
|
be usable on any *POSIX* compatible operating system. |
|
|
|
So *NO* Linux, BSD specific syscalls there. |
|
|
|
Contribution to the project |
|
--------------------------- |
|
|
|
Since CVS is effectively dead for LTP proper, we ask that you submit |
|
patches that are git friendly and patchable. |
|
|
|
Guidelines for submitting patches are as follows: |
|
|
|
1. Use +git commit -s+ . You know you want to ;) .. (you may need to |
|
submit a correct signed-off-by line, e.g. use git config first). |
|
2. Provide a short (<= 50 character) description of the commit. |
|
3. Provide a little more terse (1-2 paragraph maximum, <= 72 character |
|
lines) description of what the commit does. |
|
4. Format the email with +git format-patch --attach+ command . |
|
|
|
Example of a commit message: |
|
|
|
------------------------------------------------------------------- |
|
Short description of my commit. |
|
|
|
This is a much longer description of my commit. Blah blah blah blah |
|
blah blah blah blah blah. |
|
|
|
Signed-off-by: John Smith <dev@null> |
|
-------------------------------------------------------------------
|
|
|