commit 32857019e6751b382822a182148fe6c24a849107
parent 0a331cdaf9397c301cd865f398cb372dcbbe5b26
Author: Martin Schanzenbach <schanzen@gnunet.org>
Date: Fri, 12 Aug 2022 07:30:26 +0200
add more of old examples
Diffstat:
1 file changed, 208 insertions(+), 1 deletion(-)
diff --git a/man_developers/style.rst b/man_developers/style.rst
@@ -150,7 +150,214 @@ Note that :c:`char *` is different from :c:`const char*` and
Each variable type should be chosen with care.
-.. todo:: There is still a chunk missing here from the old docs!
+While ``goto`` should generally be avoided, having a ``goto`` to the
+end of a function to a block of clean up statements (free, close,
+etc.) can be acceptable.
+
+Conditions should be written with constants on the left (to avoid
+accidental assignment) and with the ``true`` target being either the
+``error`` case or the significantly simpler continuation. For
+example:
+
+::
+
+ if (0 != stat ("filename,"
+ &sbuf))
+ {
+ error();
+ }
+ else
+ {
+ /* handle normal case here */
+ }
+
+instead of
+
+::
+
+ if (stat ("filename," &sbuf) == 0) {
+ /* handle normal case here */
+ } else {
+ error();
+ }
+
+If possible, the error clause should be terminated with a ``return``
+(or ``goto`` to some cleanup routine) and in this case, the ``else``
+clause should be omitted:
+
+::
+
+ if (0 != stat ("filename",
+ &sbuf))
+ {
+ error();
+ return;
+ }
+ /* handle normal case here */
+
+This serves to avoid deep nesting. The 'constants on the left' rule
+applies to all constants (including. ``GNUNET_SCHEDULER_NO_TASK``),
+NULL, and enums). With the two above rules (constants on left, errors
+in 'true' branch), there is only one way to write most branches
+correctly.
+
+Combined assignments and tests are allowed if they do not hinder code
+clarity. For example, one can write:
+
+::
+
+ if (NULL == (value = lookup_function()))
+ {
+ error();
+ return;
+ }
+
+Use ``break`` and ``continue`` wherever possible to avoid deep(er)
+nesting. Thus, we would write:
+
+::
+
+ next = head;
+ while (NULL != (pos = next))
+ {
+ next = pos->next;
+ if (! should_free (pos))
+ continue;
+ GNUNET_CONTAINER_DLL_remove (head,
+ tail,
+ pos);
+ GNUNET_free (pos);
+ }
+
+instead of
+
+::
+
+ next = head; while (NULL != (pos = next)) {
+ next = pos->next;
+ if (should_free (pos)) {
+ /* unnecessary nesting! */
+ GNUNET_CONTAINER_DLL_remove (head, tail, pos);
+ GNUNET_free (pos);
+ }
+ }
+
+We primarily use ``for`` and ``while`` loops. A ``while`` loop is
+used if the method for advancing in the loop is not a straightforward
+increment operation. In particular, we use:
+
+::
+
+ next = head;
+ while (NULL != (pos = next))
+ {
+ next = pos->next;
+ if (! should_free (pos))
+ continue;
+ GNUNET_CONTAINER_DLL_remove (head,
+ tail,
+ pos);
+ GNUNET_free (pos);
+ }
+
+to free entries in a list (as the iteration changes the structure of
+the list due to the free; the equivalent ``for`` loop does no longer
+follow the simple ``for`` paradigm of ``for(INIT;TEST;INC)``).
+However, for loops that do follow the simple ``for`` paradigm we do
+use ``for``, even if it involves linked lists:
+
+::
+
+ /* simple iteration over a linked list */
+ for (pos = head;
+ NULL != pos;
+ pos = pos->next)
+ {
+ use (pos);
+ }
+
+The first argument to all higher-order functions in GNUnet must be
+declared to be of type ``void *`` and is reserved for a closure. We
+do not use inner functions, as trampolines would conflict with setups
+that use non-executable stacks. The first statement in a higher-order
+function, which unusually should be part of the variable
+declarations, should assign the ``cls`` argument to the precise
+expected type. For example:
+
+::
+
+ int
+ callback (void *cls,
+ char *args)
+ {
+ struct Foo *foo = cls;
+ int other_variables;
+
+ /* rest of function */
+ }
+
+As shown in the example above, after the return type of a function
+there should be a break. Each parameter should be on a new line.
+
+It is good practice to write complex ``if`` expressions instead of
+using deeply nested ``if`` statements. However, except for addition
+and multiplication, all operators should use parens. This is fine:
+
+::
+
+ if ( (1 == foo) ||
+ ( (0 == bar) &&
+ (x != y) ) )
+ return x;
+
+However, this is not:
+
+::
+
+ if (1 == foo)
+ return x;
+ if (0 == bar && x != y)
+ return x;
+
+Note that splitting the ``if`` statement above is debatable as the
+``return x`` is a very trivial statement. However, once the logic
+after the branch becomes more complicated (and is still identical),
+the \"or\" formulation should be used for sure.
+
+There should be two empty lines between the end of the function and
+the comments describing the following function. There should be a
+single empty line after the initial variable declarations of a
+function. If a function has no local variables, there should be no
+initial empty line. If a long function consists of several complex
+steps, those steps might be separated by an empty line (possibly
+followed by a comment describing the following step). The code should
+not contain empty lines in arbitrary places; if in doubt, it is
+likely better to NOT have an empty line (this way, more code will fit
+on the screen).
+
+When command-line arguments become too long (and would result in some
+particularly ugly ``uncrustify`` wrapping), we start all arguments on
+a new line. As a result, there must never be a new line within an
+argument declaration (i.e. between ``struct`` and the struct's name)
+or between the type and the variable). Example:
+
+::
+
+ struct GNUNET_TRANSPORT_CommunicatorHandle *
+ GNUNET_TRANSPORT_communicator_connect (
+ const struct GNUNET_CONFIGURATION_Handle *cfg,
+ const char *config_section_name,
+ const char *addr_prefix,
+ ...);
+
+Note that for short function names and arguments, the first argument
+does remain on the same line. Example:
+
+::
+
+ void
+ fun (short i,
+ short j);
Continuous integration