From 076830c068fb39bbc3e863c89a4253cbea36357e Mon Sep 17 00:00:00 2001
From: Vijay Bellur <vijay@gluster.com>
Date: Mon, 9 Apr 2012 23:11:52 +0530
Subject: doc: Move outdated documentation to legacy

Change-Id: I0ceba9a993e8b1cdef4ff6a784bfd69c08107d88
BUG: 811311
Signed-off-by: Vijay Bellur <vijay@gluster.com>
Reviewed-on: http://review.gluster.com/3116
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Amar Tumballi <amarts@redhat.com>
Reviewed-by: Anand Avati <avati@redhat.com>
---
 doc/legacy/coding-standard.tex | 385 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 385 insertions(+)
 create mode 100644 doc/legacy/coding-standard.tex

(limited to 'doc/legacy/coding-standard.tex')

diff --git a/doc/legacy/coding-standard.tex b/doc/legacy/coding-standard.tex
new file mode 100644
index 00000000000..abaedb69c3c
--- /dev/null
+++ b/doc/legacy/coding-standard.tex
@@ -0,0 +1,385 @@
+\documentclass{article}[12pt]
+\usepackage{color}
+
+\begin{document}
+
+
+\hrule
+\begin{center}\textbf{\Large{GlusterFS Coding Standards}}\end{center}
+\begin{center}\textbf{\large{\textcolor{red}{Z} Research}}\end{center}
+\begin{center}{July 14, 2008}\end{center}
+\hrule
+
+\vspace{8ex}
+
+\section*{$\bullet$ Structure definitions should have a comment per member}
+
+Every member in a structure definition must have a comment about its
+purpose. The comment should be descriptive without being overly verbose.
+
+\vspace{2ex}
+\textsl{Bad}:
+
+\begin{verbatim}
+        gf_lock_t   lock;           /* lock */
+\end{verbatim}
+
+\textsl{Good}:
+
+\begin{verbatim}
+        DBTYPE      access_mode;    /* access mode for accessing
+                                     * the databases, can be
+                                     * DB_HASH, DB_BTREE
+                                     * (option access-mode <mode>)
+                                     */
+\end{verbatim}
+
+\section*{$\bullet$ Declare all variables at the beginning of the function}
+All local variables in a function must be declared immediately after the
+opening brace. This makes it easy to keep track of memory that needs to be freed
+during exit. It also helps debugging, since gdb cannot handle variables
+declared inside loops or other such blocks.
+
+\section*{$\bullet$ Always initialize local variables}
+Every local variable should be initialized to a sensible default value
+at the point of its declaration. All pointers should be initialized to NULL,
+and all integers should be zero or (if it makes sense) an error value.
+
+\vspace{2ex}
+
+\textsl{Good}:
+
+\begin{verbatim}
+        int ret       = 0;
+        char *databuf = NULL;
+        int _fd       = -1;
+\end{verbatim}
+
+\section*{$\bullet$ Initialization should always be done with a constant value}
+Never use a non-constant expression as the initialization value for a variable.
+
+\vspace{2ex}
+
+\textsl{Bad}:
+
+\begin{verbatim}
+        pid_t pid     = frame->root->pid;
+        char *databuf = malloc (1024);
+\end{verbatim}
+
+\section*{$\bullet$ Validate all arguments to a function}
+All pointer arguments to a function must be checked for \texttt{NULL}.
+A macro named \texttt{VALIDATE} (in \texttt{common-utils.h})
+takes one argument, and if it is \texttt{NULL}, writes a log message and
+jumps to a label called \texttt{err} after setting op\_ret and op\_errno
+appropriately. It is recommended to use this template.
+
+\vspace{2ex}
+
+\textsl{Good}:
+
+\begin{verbatim}
+        VALIDATE(frame);
+        VALIDATE(this);
+        VALIDATE(inode);
+\end{verbatim}
+
+\section*{$\bullet$ Never rely on precedence of operators}
+Never write code that relies on the precedence of operators to execute
+correctly.  Such code can be hard to read and someone else might not
+know the precedence of operators as accurately as you do.
+\vspace{2ex}
+
+\textsl{Bad}:
+
+\begin{verbatim}
+        if (op_ret == -1 && errno != ENOENT)
+\end{verbatim}
+
+\textsl{Good}:
+
+\begin{verbatim}
+        if ((op_ret == -1) && (errno != ENOENT))
+\end{verbatim}
+
+\section*{$\bullet$ Use exactly matching types}
+Use a variable of the exact type declared in the manual to hold the
+return value of a function. Do not use an ``equivalent'' type.
+
+\vspace{2ex}
+
+\textsl{Bad}:
+
+\begin{verbatim}
+        int len = strlen (path);
+\end{verbatim}
+
+\textsl{Good}:
+
+\begin{verbatim}
+        size_t len = strlen (path);
+\end{verbatim}
+
+\section*{$\bullet$ Never write code such as \texttt{foo->bar->baz}; check every pointer}
+Do not write code that blindly follows a chain of pointer
+references. Any pointer in the chain may be \texttt{NULL} and thus
+cause a crash. Verify that each pointer is non-null before following
+it.
+
+\section*{$\bullet$ Check return value of all functions and system calls}
+The return value of all system calls and API functions must be checked
+for success or failure.
+
+\vspace{2ex}
+\textsl{Bad}:
+
+\begin{verbatim}
+        close (fd);
+\end{verbatim}
+
+\textsl{Good}:
+
+\begin{verbatim}
+        op_ret = close (_fd);
+        if (op_ret == -1) {
+                gf_log (this->name, GF_LOG_ERROR,
+                        "close on file %s failed (%s)", real_path,
+                        strerror (errno));
+                op_errno = errno;
+                goto out;
+        }
+\end{verbatim}
+
+
+\section*{$\bullet$ Gracefully handle failure of malloc}
+GlusterFS should never crash or exit due to lack of memory. If a
+memory allocation fails, the call should be unwound and an error
+returned to the user.
+
+\section*{$\bullet$ Use result args and reserve the return value to indicate success or failure}
+The return value of every functions must indicate success or failure (unless
+it is impossible for the function to fail --- e.g., boolean functions). If
+the function needs to return additional data, it must be returned using a
+result (pointer) argument.
+
+\vspace{2ex}
+\textsl{Bad}:
+
+\begin{verbatim}
+        int32_t dict_get_int32 (dict_t *this, char *key);
+\end{verbatim}
+
+\textsl{Good}:
+
+\begin{verbatim}
+        int dict_get_int32 (dict_t *this, char *key, int32_t *val);
+\end{verbatim}
+
+\section*{$\bullet$ Always use the `n' versions of string functions}
+Unless impossible, use the length-limited versions of the string functions.
+
+\vspace{2ex}
+\textsl{Bad}:
+
+\begin{verbatim}
+        strcpy (entry_path, real_path);
+\end{verbatim}
+
+\textsl{Good}:
+
+\begin{verbatim}
+        strncpy (entry_path, real_path, entry_path_len);
+\end{verbatim}
+
+\section*{$\bullet$ No dead or commented code}
+There must be no dead code (code to which control can never be passed) or
+commented out code in the codebase.
+
+\section*{$\bullet$ Only one unwind and return per function}
+There must be only one exit out of a function. \texttt{UNWIND} and return
+should happen at only point in the function.
+
+\section*{$\bullet$ Function length or Keep functions small}
+We live in the UNIX-world where modules do one thing and do it well.
+This rule should apply to our functions also. If a function is very long, try splitting it
+into many little helper functions. The question is, in a coding
+spree, how do we know a function is long and unreadable. One rule of
+thumb given by Linus Torvalds is that, a function should be broken-up
+if you have 4 or more levels of indentation going on for more than 3-4
+lines.
+
+\vspace{2ex}
+\textsl{Example for a helper function}:
+\begin{verbatim}
+        static int
+        same_owner (posix_lock_t *l1, posix_lock_t *l2)
+        {
+                return ((l1->client_pid == l2->client_pid) &&
+                       (l1->transport  == l2->transport));
+        }
+\end{verbatim}
+
+\section*{$\bullet$ Defining functions as static}
+Define internal functions as static only if you're
+very sure that there will not be a crash(..of any kind..) emanating in
+that function. If there is even a remote possibility, perhaps due to
+pointer derefering, etc, declare the function as non-static. This
+ensures that when a crash does happen, the function name shows up the
+in the back-trace generated by libc. However, doing so has potential
+for polluting the function namespace, so to avoid conflicts with other
+components in other parts, ensure that the function names are
+prepended with a prefix that identify the component to which it
+belongs. For eg. non-static functions in io-threads translator start
+with iot\_.
+
+\section*{$\bullet$ Ensure function calls wrap around after
+80-columns.}
+Place remaining arguments on the next line if needed.
+
+\section*{$\bullet$ Functions arguments and function definition}
+Place all the arguments of a function definition on the same line
+until the line goes beyond 80-cols. Arguments that extend beyind
+80-cols should be placed on the next line.
+
+\section*{Style issues}
+
+\subsection*{Brace placement}
+Use K\&R/Linux style of brace placement for blocks.
+
+\textsl{Example}:
+\begin{verbatim}
+        int some_function (...)
+        {
+                if (...) {
+                        /* ... */
+                } else if (...) {
+                        /* ... */
+                } else {
+                        /* ... */
+                }
+
+                do {
+                        /* ... */
+                } while (cond);
+        }
+\end{verbatim}
+
+\subsection*{Indentation}
+Use \textbf{eight} spaces for indenting blocks. Ensure that your
+file contains only spaces and not tab characters. You can do this
+in Emacs by selecting the entire file (\texttt{C-x h}) and
+running \texttt{M-x untabify}.
+
+To make Emacs indent lines automatically by eight spaces, add this
+line to your \texttt{.emacs}:
+
+\begin{verbatim}
+        (add-hook 'c-mode-hook (lambda () (c-set-style "linux")))
+\end{verbatim}
+
+\subsection*{Comments}
+Write a comment before every function describing its purpose (one-line),
+its arguments, and its return value. Mention whether it is an internal
+function or an exported function.
+
+Write a comment before every structure describing its purpose, and
+write comments about each of its members.
+
+Follow the style shown below for comments, since such comments
+can then be automatically extracted by doxygen to generate
+documentation.
+
+\textsl{Example}:
+\begin{verbatim}
+/**
+ * hash_name -hash function for filenames
+ * @par:  parent inode number
+ * @name: basename of inode
+ * @mod:  number of buckets in the hashtable
+ *
+ * @return: success: bucket number
+ *          failure: -1
+ *
+ * Not for external use.
+ */
+\end{verbatim}
+
+\subsection*{Indicating critical sections}
+To clearly show regions of code which execute with locks held, use
+the following format:
+
+\begin{verbatim}
+        pthread_mutex_lock (&mutex);
+        {
+                /* code */
+        }
+        pthread_mutex_unlock (&mutex);
+\end{verbatim}
+
+\section*{A skeleton fop function}
+This is the recommended template for any fop. In the beginning come
+the initializations. After that, the `success' control flow should be
+linear.  Any error conditions should cause a \texttt{goto} to a single
+point, \texttt{out}.  At that point, the code should detect the error
+that has occured and do appropriate cleanup.
+
+\begin{verbatim}
+int32_t
+sample_fop (call_frame_t *frame, xlator_t *this, ...)
+{
+        char *            var1     = NULL;
+        int32_t           op_ret   = -1;
+        int32_t           op_errno = 0;
+        DIR *             dir      = NULL;
+        struct posix_fd * pfd      = NULL;
+
+        VALIDATE_OR_GOTO (frame, out);
+        VALIDATE_OR_GOTO (this, out);
+
+        /* other validations */
+
+        dir = opendir (...);
+
+        if (dir == NULL) {
+                op_errno = errno;
+                gf_log (this->name, GF_LOG_ERROR,
+                        "opendir failed on %s (%s)", loc->path,
+                        strerror (op_errno));
+                goto out;
+        }
+
+        /* another system call */
+        if (...) {
+                op_errno = ENOMEM;
+                gf_log (this->name, GF_LOG_ERROR,
+                        "out of memory :(");
+                goto out;
+        }
+
+        /* ... */
+
+ out:
+        if (op_ret == -1) {
+
+          /* check for all the cleanup that needs to be
+             done */
+
+                if (dir) {
+                        closedir (dir);
+                        dir = NULL;
+                }
+
+                if (pfd) {
+                        if (pfd->path)
+                                FREE (pfd->path);
+                        FREE (pfd);
+                        pfd = NULL;
+                }
+        }
+
+        STACK_UNWIND (frame, op_ret, op_errno, fd);
+        return 0;
+}
+\end{verbatim}
+
+\end{document}
-- 
cgit 

