1Coding Style for the Xen Hypervisor
2===================================
3
4The Xen coding style described below is the coding style used by the
5Xen hypervisor itself (xen/*) as well as various associated low-level
6libraries (e.g. tools/libxc/*).
7
8An exception is made for files which are imported from an external
9source. In these cases the prevailing coding style of the upstream
10source is generally used (commonly the Linux coding style).
11
12Other parts of the code base may use other coding styles, sometimes
13explicitly (e.g. tools/libxl/CODING_STYLE) but often implicitly (Linux
14coding style is fairly common). In general you should copy the style
15of the surrounding code. If you are unsure please ask.
16
17Indentation
18-----------
19
20Indenting uses spaces, not tabs - in contrast to Linux.  An indent
21level consists of four spaces.  Code within blocks is indented by one
22extra indent level.  The enclosing braces of a block are indented the
23same as the code _outside_ the block.  e.g.
24
25void fun(void)
26{
27    /* One level of indent. */
28
29    {
30        /* A second level of indent. */
31    }
32}
33
34White space
35-----------
36
37Space characters are used to spread out logical statements, such as in
38the condition of an if or while.  Spaces are placed between the
39keyword and the brackets surrounding the condition, between the
40brackets and the condition itself, and around binary operators (except
41the structure access operators, '.' and '->'). e.g.
42
43if ( (wibble & wombat) == 42 )
44{
45    ...
46
47There should be no trailing white space at the end of lines (including
48after the opening /* of a comment block).
49
50Line Length
51-----------
52
53Lines should be less than 80 characters in length.  Long lines should
54be split at sensible places and the trailing portions indented.
55
56User visible strings (e.g., printk() messages) should not be split so
57they can searched for more easily.
58
59Bracing
60-------
61
62Braces ('{' and '}') are usually placed on a line of their own, except
63for the do/while loop.  This is unlike the Linux coding style and
64unlike K&R.  do/while loops are an exception. e.g.:
65
66if ( condition )
67{
68    /* Do stuff. */
69}
70else
71{
72    /* Other stuff. */
73}
74
75while ( condition )
76{
77    /* Do stuff. */
78}
79
80do {
81    /* Do stuff. */
82} while ( condition );
83
84etc.
85
86Braces should be omitted for blocks with a single statement. e.g.,
87
88if ( condition )
89    single_statement();
90
91Types
92-----
93
94Use basic C types and C standard mandated typedef-s where possible (and
95with preference in this order).  This in particular means to avoid u8,
96u16, etc despite those types continuing to exist in our code base.
97Fixed width types should only be used when a fixed width quantity is
98meant (which for example may be a value read from or to be written to a
99register).
100
101Especially with pointer types, whenever the pointed to object is not
102(supposed to be) modified, qualify the pointed to type with "const".
103
104Comments
105--------
106
107Only C style /* ... */ comments are to be used.  C++ style // comments
108should not be used.  Multi-word comments should begin with a capital
109letter.  Comments containing a single sentence may end with a full
110stop; comments containing several sentences must have a full stop
111after each sentence.
112
113Multi-line comment blocks should start and end with comment markers on
114separate lines and each line should begin with a leading '*'.
115
116/*
117 * Example, multi-line comment block.
118 *
119 * Note beginning and end markers on separate lines and leading '*'.
120 */
121
122Emacs local variables
123---------------------
124
125A comment block containing local variables for emacs is permitted at
126the end of files.  It should be:
127
128/*
129 * Local variables:
130 * mode: C
131 * c-file-style: "BSD"
132 * c-basic-offset: 4
133 * indent-tabs-mode: nil
134 * End:
135 */
136
137Handling unexpected conditions
138------------------------------
139
140GUIDELINES:
141
142Passing errors up the stack should be used when the caller is already
143expecting to handle errors, and the state when the error was
144discovered isn’t broken, or isn't too hard to fix.
145
146domain_crash() should be used when passing errors up the stack is too
147difficult, and/or when fixing up state of a guest is impractical, but
148where fixing up the state of Xen will allow Xen to continue running.
149This is particularly appropriate when the guest is exhibiting behavior
150well-behaved guests shouldn't.
151
152BUG_ON() should be used when you can’t pass errors up the stack, and
153either continuing or crashing the guest would likely cause an
154information leak or privilege escalation vulnerability.
155
156ASSERT() IS NOT AN ERROR HANDLING MECHANISM.  ASSERT is a way to move
157detection of a bug earlier in the programming cycle; it is a
158more-noticeable printk.  It should only be added after one of the
159other three error-handling mechanisms has been evaluated for
160reliability and security.
161
162RATIONALE:
163
164It's frequently the case that code is written with the assumption that
165certain conditions can never happen.  There are several possible
166actions programmers can take in these situations:
167
168* Programmers can simply not handle those cases in any way, other than
169perhaps to write a comment documenting what the assumption is.
170
171* Programmers can try to handle the case gracefully -- fixing up
172in-progress state and returning an error to the user.
173
174* Programmers can crash the guest.
175
176* Programmers can use ASSERT(), which will cause the check to be
177executed in DEBUG builds, and cause the hypervisor to crash if it's
178violated
179
180* Programmers can use BUG_ON(), which will cause the check to be
181executed in both DEBUG and non-DEBUG builds, and cause the hypervisor
182to crash if it's violated.
183
184In selecting which response to use, we want to achieve several goals:
185
186- To minimize risk of introducing security vulnerabilities,
187  particularly as the code evolves over time
188
189- To efficiently spend programmer time
190
191- To detect violations of assumptions as early as possible
192
193- To minimize the impact of bugs on production use cases
194
195The guidelines above attempt to balance these:
196
197- When the caller is expecting to handle errors, and there is no
198broken state at the time the unexpected condition is discovered, or
199when fixing the state is straightforward, then fixing up the state and
200returning an error is the most robust thing to do.  However, if the
201caller isn't expecting to handle errors, or if the state is difficult
202to fix, then returning an error may require extensive refactoring,
203which is not a good use of programmer time when they're certain that
204this condition cannot occur.
205
206- BUG_ON() will stop all hypervisor action immediately.  In situations
207where continuing might allow an attacker to escalate privilege, a
208BUG_ON() can change a privilege escalation or information leak into a
209denial-of-service (an improvement).  But in situations where
210continuing (say, returning an error) might be safe, then BUG_ON() can
211change a benign failure into denial-of-service (a degradation).
212
213- domain_crash() is similar to BUG_ON(), but with a more limited
214effect: it stops that domain immediately.  In situations where
215continuing might cause guest or hypervisor corruption, but destroying
216the guest allows the hypervisor to continue, this can change a more
217serious bug into a guest denial-of-service.  But in situations where
218returning an error might be safe, then domain_crash() can change a
219benign failure into a guest denial-of-service.
220
221- ASSERT() will stop the hypervisor during development, but allow
222hypervisor action to continue during production.  In situations where
223continuing will at worst result in a denial-of-service, and at best
224may have little effect other than perhaps quirky behavior, using an
225ASSERT() will allow violation of assumptions to be detected as soon as
226possible, while not causing undue degradation in production
227hypervisors.  However, in situations where continuing could cause
228privilege escalation or information leaks, using an ASSERT() can
229introduce security vulnerabilities.
230
231Note however that domain_crash() has its own traps: callers far up the
232call stack may not realize that the domain is now dying as a result of
233an innocuous-looking operation, particularly if somewhere on the
234callstack between the initial function call and the failure, no error
235is returned.  Using domain_crash() requires careful inspection and
236documentation of the code to make sure all callers at the stack handle
237a newly-dead domain gracefully.
238