diff options
author | Vijay Bellur <vijay@gluster.com> | 2012-04-09 23:11:52 +0530 |
---|---|---|
committer | Anand Avati <avati@redhat.com> | 2012-04-11 10:25:56 -0700 |
commit | 076830c068fb39bbc3e863c89a4253cbea36357e (patch) | |
tree | 842884d8db9a40d5a53e5171c852a84daa8e0f65 /doc/coding-standard.tex | |
parent | df8e2f53b70f4f49af70df308010dddfe5ca35ec (diff) |
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>
Diffstat (limited to 'doc/coding-standard.tex')
-rw-r--r-- | doc/coding-standard.tex | 385 |
1 files changed, 0 insertions, 385 deletions
diff --git a/doc/coding-standard.tex b/doc/coding-standard.tex deleted file mode 100644 index 92f799c013f..00000000000 --- a/doc/coding-standard.tex +++ /dev/null @@ -1,385 +0,0 @@ -\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} |