aboutsummaryrefslogtreecommitdiffstats
path: root/doc/CODING-GUIDELINES
blob: 44063bd1febd05526088e0e488cad2e31be54246 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
573
574
575
576
577
578
579
580
581
582
583
584
585
586
587
588
589
590
591
592
593
594
595
596
597
598
599
600
601
602
603
604
605
606
607
608
609
610
611
612
613
614
615
616
617
618
619
620
621
622
623
624
625
626
627
628
629
630
631
632
633
634
635
636
637
638
639
640
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655
656
657
658
659
660
661
662
663
664
665
666
667
668
669
670
671
672
673
674
675
676
677
678
679
680
681
682
683
684
685
686
687
688
689
690
691
692
693
694
695
696
697
698
699
700
701
702
703
704
705
706
707
708
709
710
711
712
713
714
715
716
717
718
719
720
721
722
723
724
725
726
727
728
729
730
731
732
733
734
735
736
737
738
739
740
741
742
743
744
745
746
747
748
749
750
751
752
753
754
755
756
757
758
759
760
761
762
763
764
765
766
767
768
769
770
771
772
773
774
775
776
777
778
779
780
781
782
783
784
785
786
787
788
789
790
791
792
793
794
795
796
797
798
799
800
801
802
803
804
805
806
807
808
809
810
811
812
813
814
815
816
817
818
819
820
821
822
823
824
825
826
827
828
829
830
831
832
833
834
835
836
837
838
839
840
841
842
843
844
845
846
847
848
849
850
851
852
853
854
855
856
857
858
859
860
861
862
863
864
865
866
867
868
869
870
871
872
873
874
875
876
877
878
879
880
881
882
883
884
885
886
887
888
889
890
891
892
893
894
895
896
897
898
899
900
901
902
903
904
905
906
907
908
909
910
911
912
913
914
915
916
917
918
919
920
921
922
923
924
925
926
927
928
929
930
931
932
	    --------------------------------------
	    == Asterisk Coding Guidelines ==
	    --------------------------------------

This document gives some basic indication on how the asterisk code
is structured. The first part covers the structure and style of
individual files. The second part (TO BE COMPLETED) covers the
overall code structure and the build architecture.

Please read it to the end to understand in detail how the asterisk
code is organized, and to know how to extend asterisk or contribute
new code.

We are looking forward to your contributions to Asterisk - the 
Open Source PBX! As Asterisk is a large and in some parts very
time-sensitive application, the code base needs to conform to
a common set of coding rules so that many developers can enhance
and maintain the code. Code also needs to be reviewed and tested
so that it works and follows the general architecture and guide-
lines, and is well documented.

Asterisk is published under a dual-licensing scheme by Digium.
To be accepted into the codebase, all non-trivial changes must be
disclaimed to Digium or placed in the public domain. For more information
see http://bugs.digium.com

Patches should be in the form of a unified (-u) diff, made from a checkout
from subversion.

/usr/src/asterisk$ svn diff > mypatch

If you would like to only include changes to certain files in the patch, you
can list them in the "svn diff" command:

/usr/src/asterisk$ svn diff somefile.c someotherfile.c > mypatch

		-----------------------------------
		==  PART ONE: CODING GUIDELINES  ==
		-----------------------------------

* General rules
---------------

- All code, filenames, function names and comments must be in ENGLISH.

- Don't annotate your changes with comments like "/* JMG 4/20/04 */";
  Comments should explain what the code does, not when something was changed
  or who changed it. If you have done a larger contribution, make sure
  that you are added to the CREDITS file.

- Don't make unnecessary whitespace changes throughout the code.
  If you make changes, submit them to the tracker as separate patches
  that only include whitespace and formatting changes.

- Don't use C++ type (//) comments.

- Try to match the existing formatting of the file you are working on.

- Use spaces instead of tabs when aligning in-line comments or #defines (this makes 
  your comments aligned even if the code is viewed with another tabsize)

* File structure and header inclusion
-------------------------------------

Every C source file should start with a proper copyright
and a brief description of the content of the file.
Following that, you should immediately put the following lines:

#include "asterisk.h"
ASTERISK_FILE_VERSION(__FILE__, "$Revision$")

"asterisk.h" resolves OS and compiler dependencies for the basic
set of unix functions (data types, system calls, basic I/O
libraries) and the basic Asterisk APIs.
ASTERISK_FILE_VERSION() stores in the executable information
about the file.   

Next, you should #include extra headers according to the functionality
that your file uses or implements. For each group of functions that
you use there is a common header, which covers OS header dependencies
and defines the 'external' API of those functions (the equivalent
of 'public' members of a class).  As an example:

    asterisk/module.h
        if you are implementing a module, this should be included in one
        of the files that are linked with the module.

    asterisk/io.h
        access to extra file I/O functions (stat, fstat, playing with
        directories etc)

    asterisk/network.h
        basic network I/O - all of the socket library, select/poll,
        and asterisk-specific (usually either thread-safe or reentrant
        or both) functions to play with socket addresses.

    asterisk/app.h
        parsing of application arguments

    asterisk/channel.h
        struct ast_channel and functions to manipulate it

For more information look at the headers in include/asterisk/ .
These files are usually self-sufficient, i.e. they recursively #include
all the extra headers they need.

The equivalent of 'private' members of a class are either directly in
the C source file, or in files named asterisk/mod_*.h to make it clear
that they are not for inclusion by generic code.

Keep the number of header files small by not including them unnecessarily.
Don't cut&paste list of header files from other sources, but only include
those you really need. Apart from obvious cases (e.g. module.h which
is almost always necessary) write a short comment next to each #include to
explain why you need it.


* Declaration of functions and variables
----------------------------------------

- Do not declare variables mid-block (e.g. like recent GNU compilers support)
  since it is harder to read and not portable to GCC 2.95 and others.

- Functions and variables that are not intended to be used outside the module
  must be declared static.

- When reading integer numeric input with scanf (or variants), do _NOT_ use '%i'
  unless you specifically want to allow non-base-10 input; '%d' is always a better
  choice, since it will not silently turn numbers with leading zeros into base-8.

- Strings that are coming from input should not be used as a first argument to
  a formatted *printf function.

* Use the internal API
----------------------

- Make sure you are aware of the string and data handling functions that exist
  within Asterisk to enhance portability and in some cases to produce more
  secure and thread-safe code. Check utils.c/utils.h for these.

- If you need to create a detached thread, use the ast_pthread_create_detached()
  normally or ast_pthread_create_detached_background() for a thread with a smaller
  stack size.  This reduces the replication of the code to handle the pthread_attr_t
  structure.

* Code formatting
-----------------

Roughly, Asterisk code formatting guidelines are generally equivalent to the 
following:

# indent -i4 -ts4 -br -brs -cdw -lp -ce -nbfda -npcs -nprs -npsl -nbbo -saf -sai -saw -cs -l90 foo.c

this means in verbose:
 -i4:    indent level 4
 -ts4:   tab size 4
 -br:    braces on if line
 -brs:   braces on struct decl line
 -cdw:   cuddle do while
 -lp:    line up continuation below parenthesis
 -ce:    cuddle else
 -nbfda: dont break function decl args
 -npcs:  no space after function call names
 -nprs:  no space after parentheses
 -npsl:  dont break procedure type
 -saf:   space after for
 -sai:   space after if
 -saw:   space after while
 -cs:    space after cast
 -ln90:  line length 90 columns

Function calls and arguments should be spaced in a consistent way across
the codebase.
	GOOD: foo(arg1, arg2);
	GOOD: foo(arg1,arg2);	/* Acceptable but not preferred */
	BAD: foo (arg1, arg2);
	BAD: foo( arg1, arg2 );
	BAD: foo(arg1, arg2,arg3);

Don't treat keywords (if, while, do, return) as if they were functions;
leave space between the keyword and the expression used (if any). For 'return',
don't even put parentheses around the expression, since they are not
required.

There is no shortage of whitespace characters :-) Use them when they make
the code easier to read. For example:

	for (str=foo;str;str=str->next)

is harder to read than

	for (str = foo; str; str = str->next)

Following are examples of how code should be formatted.

- Functions:
int foo(int a, char *s)
{
	return 0;
}

- If statements:
if (foo) {
	bar();
} else {
	blah();
}

- Case statements:
switch (foo) {
case BAR:
	blah();
	break;
case OTHER:
	other();
	break;
}

- No nested statements without braces, e.g.:

for (x = 0; x < 5; x++)
	if (foo) 
		if (bar)
			baz();

instead do:
for (x = 0; x < 5; x++) {
	if (foo) {
		if (bar) {
			baz();
		}
	}
}

- Always use braces around the statements following an if/for/while construct,
even if not strictly necessary, as it reduces future possible problems.

- Don't build code like this:

if (foo) {
	/* .... 50 lines of code ... */
} else {
	result = 0;
	return;
}

Instead, try to minimize the number of lines of code that need to be
indented, by only indenting the shortest case of the 'if'
statement, like so:

if (!foo) {
	result = 0;
	return;
}

.... 50 lines of code ....

When this technique is used properly, it makes functions much easier to read
and follow, especially those with more than one or two 'setup' operations
that must succeed for the rest of the function to be able to execute.

- Labels/goto are acceptable
Proper use of this technique may occasionally result in the need for a
label/goto combination so that error/failure conditions can exit the
function while still performing proper cleanup. This is not a bad thing!
Use of goto in this situation is encouraged, since it removes the need
for excess code indenting without requiring duplication of cleanup code.
       
- Never use an uninitialized variable
Make sure you never use an uninitialized variable.  The compiler will 
usually warn you if you do so. However, do not go too far the other way,
and needlessly initialize variables that do not require it. If the first
time you use a variable in a function is to store a value there, then
initializing it at declaration is pointless, and will generate extra
object code and data in the resulting binary with no purpose. When in doubt,
trust the compiler to tell you when you need to initialize a variable;
if it does not warn you, initialization is not needed.

- Do not cast 'void *'
Do not explicitly cast 'void *' into any other type, nor should you cast any
other type into 'void *'. Implicit casts to/from 'void *' are explicitly
allowed by the C specification. This means the results of malloc(), calloc(),
alloca(), and similar functions do not _ever_ need to be cast to a specific
type, and when you are passing a pointer to (for example) a callback function
that accepts a 'void *' you do not need to cast into that type.

* Function naming
-----------------

All public functions (those not marked 'static'), must be named "ast_<something>"
and have a descriptive name.

As an example, suppose you wanted to take a local function "find_feature", defined
as static in a file, and used only in that file, and make it public, and use it
in other files. You will have to remove the "static" declaration and define a 
prototype in an appropriate header file (usually in include/asterisk). A more
specific name should be given, such as "ast_find_call_feature".

* Variable naming
-----------------

- Global variables
Name global variables (or local variables when you have a lot of them or
are in a long function) something that will make sense to aliens who
find your code in 100 years.  All variable names should be in lower 
case, except when following external APIs or specifications that normally
use upper- or mixed-case variable names; in that situation, it is
preferable to follow the external API/specification for ease of
understanding.

Make some indication in the name of global variables which represent
options that they are in fact intended to be global.
 e.g.: static char global_something[80]

- Don't use un-necessary typedef's
Don't use 'typedef' just to shorten the amount of typing; there is no substantial
benefit in this:
struct foo { int bar; }; typedef struct foo foo_t;

In fact, don't use 'variable type' suffixes at all; it's much preferable to
just type 'struct foo' rather than 'foo_s'.

- Use enums instead of #define where possible
Use enums rather than long lists of #define-d numeric constants when possible;
this allows structure members, local variables and function arguments to
be declared as using the enum's type. For example:

enum option {
  OPT_FOO = 1
  OPT_BAR = 2
  OPT_BAZ = 4
};

static enum option global_option;

static handle_option(const enum option opt)
{
  ...
}

Note: The compiler will _not_ force you to pass an entry from the enum
as an argument to this function; this recommendation serves only to make
the code clearer and somewhat self-documenting. In addition, when using
switch/case blocks that switch on enum values, the compiler will warn
you if you forget to handle one or more of the enum values, which can be
handy.

* String handling
-----------------

Don't use strncpy for copying whole strings; it does not guarantee that the
output buffer will be null-terminated. Use ast_copy_string instead, which
is also slightly more efficient (and allows passing the actual buffer
size, which makes the code clearer).

Don't use ast_copy_string (or any length-limited copy function) for copying
fixed (known at compile time) strings into buffers, if the buffer is something
that has been allocated in the function doing the copying. In that case, you
know at the time you are writing the code whether the buffer is large enough
for the fixed string or not, and if it's not, your code won't work anyway!
Use strcpy() for this operation, or directly set the first two characters
of the buffer if you are just trying to store a one-character string in the
buffer. If you are trying to 'empty' the buffer, just store a single
NULL character ('\0') in the first byte of the buffer; nothing else is
needed, and any other method is wasteful.

In addition, if the previous operations in the function have already
determined that the buffer in use is adequately sized to hold the string
you wish to put into it (even if you did not allocate the buffer yourself),
use a direct strcpy(), as it can be inlined and optimized to simple
processor operations, unlike ast_copy_string().

* String conversions
--------------------

When converting from strings to integers or floats, use the sscanf function
in preference to the atoi and atof family of functions, as sscanf detects
errors.  Always check the return value of sscanf to verify that your numeric
variables successfully scanned before using them.  Also, to avoid a potential
libc bug, always specify a maximum width for each conversion specifier,
including integers and floats.  A good length for both integers and floats is
30, as this is more than generous, even if you're using doubles or long
integers.

* Use of functions
------------------

For the sake of uclibc, do not use index, bcopy or bzero; use 
strchr(), memset(), and memmove() instead. uclibc can be configured
to supply these functions, but we can save these users
time and consternation if we abstain from using these
functions.

When making applications, always ast_strdupa(data) to a local pointer if
you intend to parse the incoming data string.

	if (data)
		mydata = ast_strdupa(data);


- Use the argument parsing macros to declare arguments and parse them, i.e.:

	AST_DECLARE_APP_ARGS(args,
		AST_APP_ARG(arg1);
		AST_APP_ARG(arg2);
		AST_APP_ARG(arg3);
	);
	parse = ast_strdupa(data);
	AST_STANDARD_APP_ARGS(args, parse);

- Create generic code!
If you do the same or a similar operation more than one time, make it a
function or macro.

Make sure you are not duplicating any functionality already found in an
API call somewhere.  If you are duplicating functionality found in 
another static function, consider the value of creating a new API call 
which can be shared.

* Handling of pointers and allocations
--------------------------------------

- Dereference or localize pointers
Always dereference or localize pointers to things that are not yours like
channel members in a channel that is not associated with the current 
thread and for which you do not have a lock.
	channame = ast_strdupa(otherchan->name);

- Use const on pointer arguments if possible
Use const on pointer arguments which your function will not be modifying, as this 
allows the compiler to make certain optimizations. In general, use 'const'
on any argument that you have no direct intention of modifying, as it can
catch logic/typing errors in your code when you use the argument variable
in a way that you did not intend.

- Do not create your own linked list code - reuse!
As a common example of this point, make an effort to use the lockable
linked-list macros found in include/asterisk/linkedlists.h. They are
efficient, easy to use and provide every operation that should be
necessary for managing a singly-linked list (if something is missing,
let us know!). Just because you see other open-coded list implementations
in the source tree is no reason to continue making new copies of
that code... There are also a number of common string manipulation
and timeval manipulation functions in asterisk/strings.h and asterisk/time.h;
use them when possible.

- Avoid needless allocations!
Avoid needless malloc(), strdup() calls. If you only need the value in
the scope of your function try ast_strdupa() or declare structs on the
stack and pass a pointer to them. However, be careful to _never_ call
alloca(), ast_strdupa() or similar functions in the argument list
of a function you are calling; this can cause very strange stack
arrangements and produce unexpected behavior.

-Allocations for structures
When allocating/zeroing memory for a structure, use code like this:

struct foo *tmp;

...

tmp = ast_calloc(1, sizeof(*tmp));

Avoid the combination of ast_malloc() and memset().  Instead, always use
ast_calloc(). This will allocate and zero the memory in a single operation. 
In the case that uninitialized memory is acceptable, there should be a comment
in the code that states why this is the case.

Using sizeof(*tmp) instead of sizeof(struct foo) eliminates duplication of the 
'struct foo' identifier, which makes the code easier to read and also ensures 
that if it is copy-and-pasted it won't require as much editing.

The ast_* family of functions for memory allocation are functionally the same.
They just add an Asterisk log error message in the case that the allocation
fails for some reason. This eliminates the need to generate custom messages
throughout the code to log that this has occurred.

-String Duplications

The functions strdup and strndup can *not* accept a NULL argument. This results
in having code like this:

	if (str)
		newstr = strdup(str);
	else
		newstr = NULL;

However, the ast_strdup and ast_strdupa functions will happily accept a NULL
argument without generating an error.  The same code can be written as:
	
	newstr = ast_strdup(str);

Furthermore, it is unnecessary to have code that malloc/calloc's for the length
of a string (+1 for the terminating '\0') and then using strncpy to copy the
copy the string into the resulting buffer.  This is the exact same thing as
using ast_strdup.

* CLI Commands
--------------

New CLI commands should be named using the module's name, followed by a verb
and then any parameters that the command needs. For example:

*CLI> iax2 show peer <peername>

not

*CLI> show iax2 peer <peername>

* New dialplan applications/functions
-------------------------------------

There are two methods of adding functionality to the Asterisk
dialplan: applications and functions. Applications (found generally in
the apps/ directory) should be collections of code that interact with
a channel and/or user in some significant way. Functions (which can be
provided by any type of module) are used when the provided
functionality is simple... getting/retrieving a value, for
example. Functions should also be used when the operation is in no way
related to a channel (a computation or string operation, for example).

Applications are registered and invoked using the
ast_register_application function; see the apps/app_skel.c file for an
example.

Functions are registered using 'struct ast_custom_function'
structures and the ast_custom_function_register function.

* Doxygen API Documentation Guidelines
--------------------------------------

When writing Asterisk API documentation the following format should be
followed. Do not use the javadoc style.

/*!
 * \brief Do interesting stuff.
 * \param thing1 interesting parameter 1.
 * \param thing2 interesting parameter 2.
 *
 * This function does some interesting stuff.
 *
 * \return zero on success, -1 on error.
 */
int ast_interesting_stuff(int thing1, int thing2)
{
	return 0;
}

Notice the use of the \param, \brief, and \return constructs.  These should be
used to describe the corresponding pieces of the function being documented.
Also notice the blank line after the last \param directive.  All doxygen
comments must be in one /*! */ block.  If the function or struct does not need
an extended description it can be left out.

Please make sure to review the doxygen manual and make liberal use of the \a,
\code, \c, \b, \note, \li and \e modifiers as appropriate.

When documenting a 'static' function or an internal structure in a module,
use the \internal modifier to ensure that the resulting documentation
explicitly says 'for internal use only'.

Structures should be documented as follows.

/*!
 * \brief A very interesting structure.
 */
struct interesting_struct
{
	/*! \brief A data member. */
	int member1;

	int member2; /*!< \brief Another data member. */
}

Note that /*! */ blocks document the construct immediately following them
unless they are written, /*!< */, in which case they document the construct
preceding them.

It is very much preferred that documentation is not done inline, as done in
the previous example for member2.  The first reason for this is that it tends
to encourage extremely brief, and often pointless, documentation since people
try to keep the comment from making the line extremely long.  However, if you
insist on using inline comments, please indent the documentation with spaces!
That way, all of the comments are properly aligned, regardless of what tab
size is being used for viewing the code.

* Finishing up before you submit your code
------------------------------------------

- Look at the code once more
When you achieve your desired functionality, make another few refactor
passes over the code to optimize it.

- Read the patch
Before submitting a patch, *read* the actual patch file to be sure that 
all the changes you expect to be there are, and that there are no 
surprising changes you did not expect. During your development, that
part of Asterisk may have changed, so make sure you compare with the
latest CVS.

- Listen to advice
If you are asked to make changes to your patch, there is a good chance
the changes will introduce bugs, check it even more at this stage.
Also remember that the bug marshal or co-developer that adds comments 
is only human, they may be in error :-)

- Optimize, optimize, optimize
If you are going to reuse a computed value, save it in a variable
instead of recomputing it over and over.  This can prevent you from 
making a mistake in subsequent computations, making it easier to correct
if the formula has an error and may or may not help optimization but 
will at least help readability.

Just an example (so don't over analyze it, that'd be a shame):

const char *prefix = "pre";	
const char *postfix = "post";
char *newname;
char *name = "data";

if (name && (newname = alloca(strlen(name) + strlen(prefix) + strlen(postfix) + 3)))
	snprintf(newname, strlen(name) + strlen(prefix) + strlen(postfix) + 3, "%s/%s/%s", prefix, name, postfix);

...vs this alternative:

const char *prefix = "pre";
const char *postfix = "post";
char *newname;
char *name = "data";
int len = 0;

if (name && (len = strlen(name) + strlen(prefix) + strlen(postfix) + 3) && (newname = alloca(len)))
	snprintf(newname, len, "%s/%s/%s", prefix, name, postfix);

* Creating new manager events?
------------------------------
If you create new AMI events, please read manager.txt. Do not re-use
existing headers for new purposes, but please re-use existing headers
for the same type of data.

Manager events that signal a status are required to have one
event name, with a status header that shows the status.
The old style, with one event named "ThisEventOn" and another named
"ThisEventOff", is no longer approved.

Check manager.txt for more information on manager and existing
headers. Please update this file if you add new headers.

* Locking in Asterisk
-----------------------------

A) Locking Fundamentals

Asterisk is a heavily multithreaded application.  It makes extensive
use of locking to ensure safe access to shared resources between
different threads.

When more that one lock is involved in a given code path, there is the
potential for deadlocks.  A deadlock occurs when a thread is stuck
waiting for a resource that it will never acquire.  Here is a classic
example of a deadlock:

   Thread 1                   Thread 2
   ------------               ------------
   Holds Lock A               Holds Lock B
   Waiting for Lock B         Waiting for Lock A

In this case, there is a deadlock between threads 1 and 2.
This deadlock would have been avoided if both threads had
agreed that one must acquire Lock A before Lock B.

In general, the fundamental rule for dealing with multiple locks is

    an order _must_ be established to acquire locks, and then all threads
    must respect that order when acquiring locks.


A.1) Establishing a locking order

Because any ordering for acquiring locks is ok, one could establish
the rule arbitrarily, e.g. ordering by address, or by some other criterion.
The main issue, though, is defining an order that
  i) is easy to check at runtime;
  ii) reflects the order in which the code executes.
As an example, if a data structure B is only accessible through a
data structure A, and both require locking, then the natural order
is locking first A and then B.
As another example, if we have some unrelated data structures to
be locked in pairs, then a possible order can be based on the address
of the data structures themselves.

B) Minding the boundary between channel drivers and the Asterisk core

The #1 cause of deadlocks in Asterisk is by not properly following the
locking rules that exist at the boundary between Channel Drivers and
the Asterisk core.  The Asterisk core allocates an ast_channel, and
Channel Drivers allocate "technology specific private data" (PVT) that is
associated with an ast_channel.  Typically, both the ast_channel and
PVT have their own lock.  There are _many_
code paths that require both objects to be locked.

The locking order in this situation is the following:

    1) ast_channel
    2) PVT

Channel Drivers implement the ast_channel_tech interface to provide a
channel implementation for Asterisk.  Most of the channel_tech
interface callbacks are called with the associated ast_channel
locked.  When accessing technology specific data, the PVT can be locked
directly because the locking order is respected.

C) Preventing lock ordering reversals.

There are some code paths which make it extremely difficult to
respect the locking order.
Consider for example the following situation:

    1) A message comes in over the "network"
    2) The Channel Driver (CD) monitor thread receives the message
    3) The CD associates the message with a PVT and locks the PVT
    4) While processing the message, the CD must do something that requires
       locking the ast_channel associated to the PVT

This is the point that must be handled carefully.
The following psuedo-code

      unlock(pvt);
      lock(ast_channel);
      lock(pvt);

is _not_ correct for two reasons:

i) first and foremost, unlocking the PVT means that other threads
   can acquire the lock and believe it is safe to modify the
   associated data. When reacquiring the lock, the original thread
   might find unexpected changes in the protected data structures.
   This essentially means that the original thread must behave as if
   the lock on the pvt was not held, in which case it could have
   released it itself altogether;

ii) Asterisk uses the so called "recursive" locks, which allow a thread
   to issue a lock() call multiple times on the same lock. Recursive
   locks count the number of calls, and they require an equivalent
   number of unlock() to be actually released.

   For this reason, just calling unlock() once does not guarantee that the
   lock is actually released -- it all depends on how many times lock()
   was called before.

An alternative, but still incorrect, construct is widely used in
the asterisk code to try and improve the situation:

      while (trylock(ast_channel) == FAILURE) {
          unlock(pvt);
          usleep(1); /* yield to other threads */
          lock(pvt);
      }

Here the trylock() is non blocking, so we do not deadlock if the ast_channel
is already locked by someone else: in this case, we try to unlock the PVT
(which happens only if the PVT lock counter is 1), yield the CPU to
give other threads a chance to run, and then acquire the lock again.

This code is not correct for two reasons:
  i) same as in the previous example, it releases the lock when the thread
     probably did not expect it;
  ii) if the PVT lock counter is greater than 1 we will not
     really release the lock on the PVT. We might be lucky and have the
     other contender actually release the lock itself, and so we will "win"
     the race, but if both contenders have their lock counts > 1 then
     they will loop forever (basically replacing deadlock with livelock).

Another variant of this code is the following:

    if (trylock(ast_channel) == FAILURE) {
        unlock(pvt);
        lock(ast_channel);
        lock(pvt);
    }

which has the same issues as the while(trylock...) code, but just
deadlocks instead of looping forever in case of lock counts > 1.

The deadlock/livelock could be in principle spared if one had an
unlock_all() function that calls unlock as many times as needed to
actually release the lock, and reports the count. Then we could do:

    if (trylock(ast_channel) == FAILURE) {
        n = unlock_all(pvt);
        lock(ast_channel)
        while (n-- > 0) lock(pvt);
    }

The issue with unexpected unlocks remains, though.

C) Locking multiple channels.

The next situation to consider is what to do when you need a lock on
multiple ast_channels (or multiple unrelated data structures).

If we are sure that we do not hold any of these locks, then the
following construct is sufficient:

         lock(MIN(chan1, chan2));
         lock(MAX(chan1, chan2));

That type of code would follow an established locking order of always
locking the channel that has a lower address first.  Also keep in mind
that to use this construct for channel locking, one would have to go
through the entire codebase to ensure that when two channels are locked,
this locking order is used.
   However, if we enter the above section of code with some lock held
(which would be incorrect using non-recursive locks, but is completely
legal using recursive mutexes) then the locking order is not guaranteed
anymore because it depends on which locks we already hold. So we have
to go through the same tricks used for the channel+PVT case.

D) Recommendations

As you can see from the above discussion, getting locking right is all
but easy. So please follow these recommendations when using locks:

*) Use locks only when really necessary
    Please try to use locks only when strictly necessary, and only for
    the minimum amount of time required to run critical sections of code.
    A common use of locks in the current code is to protect a data structure
    from being released while you use it.
    With the use of reference-counted objects (astobj2) this should not be
    necessary anymore.

*) Do not sleep while holding a lock
    If possible, do not run any blocking code while holding a lock,
    because you will also block other threads trying to access the same
    lock. In many cases, you can hold a reference to the object to avoid
    that it is deleted while you sleep, perhaps set a flag in the object
    itself to report other threads that you have some pending work to
    complete, then release and acquire the lock around the blocking path,
    checking the status of the object after you acquire the lock to make
    sure that you can still perform the operation you wanted to.

*) Try not to exploit the 'recursive' feature of locks.
    Recursive locks are very convenient when coding, as you don't have to
    worry, when entering a section of code, whether or not you already
    hold the lock -- you can just protect the section with a lock/unlock
    pair and let the lock counter track things for you.
       But as you have seen, exploiting the features of recursive locks
    make it a lot harder to implement proper deadlock avoidance strategies.
    So please try to analyse your code and determine statically whether you
    already hold a lock when entering a section of code.
       If you need to call some function foo() with and without a lock held,
    you could define two function as below:
        foo_locked(...) {
            ... do something, assume lock held
        }

        foo(...) {
            lock(xyz)
            ret = foo_locked(...)
            unlock(xyz)
            return ret;
        }
    and call them according to the needs.

*) Document locking rules.
    Please document the locking order rules are documented for every
    lock introduced into Asterisk.  This is done almost nowhere in the
    existing code.  However, it will be expected to be there for newly
    introduced code.  Over time, this information should be added for
    all of the existing lock usage.

-----------------------------------------------------------------------


	    ------------------------------------
	    ==  PART TWO: BUILD ARCHITECTURE  ==
	    ------------------------------------

The asterisk build architecture relies on autoconf to detect the
system configuration, and on a locally developed tool (menuselect) to
select build options and modules list, and on gmake to do the build.

The first step, usually to be done soon after a checkout, is running
"./configure", which will store its findings in two files:

    + include/asterisk/autoconfig.h
	contains C macros, normally #define HAVE_FOO or HAVE_FOO_H ,
	for all functions and headers that have been detected at build time.
	These are meant to be used by C or C++ source files.

    + makeopts
	contains variables that can be used by Makefiles.
	In addition to the usual CC, LD, ... variables pointing to
	the various build tools, and prefix, includedir ... which are
	useful for generic compiler flags, there are variables
	for each package detected.
	These are normally of the form FOO_INCLUDE=... FOO_LIB=...
	FOO_DIR=... indicating, for each package, the useful libraries
	and header files. 

The next step is to run "make menuselect", to extract the dependencies existing
between files and modules, and to store build options.
menuselect produces two files, both to be read by the Makefile:

    + menuselect.makeopts
	Contains for each subdirectory a list of modules that must be
	excluded from the build, plus some additional informatiom.
    + menuselect.makedeps
	Contains, for each module, a list of packages it depends on.
	For each of these packages, we can collect the relevant INCLUDE
	and LIB files from makeopts. This file is based on information
	in the .c source code files for each module.

The top level Makefile is in charge of setting up the build environment,
creating header files with build options, and recursively invoking the
subdir Makefiles to produce modules and the main executable.

The sources are split in multiple directories, more or less divided by
module type (apps/ channels/ funcs/ res/ ...) or by function, for the main
binary (main/ pbx/).


TO BE COMPLETED

    
-----------------------------------------------
Welcome to the Asterisk development community!
Meet you on the asterisk-dev mailing list. 
Subscribe at http://lists.digium.com!

Mark Spencer, Kevin P. Fleming and 
the Asterisk.org Development Team