mirror of
https://git.openafs.org/openafs.git
synced 2025-01-18 15:00:12 +00:00
f2f5a7bca5
Update the style guide with a declaration of the prevailing and preferred brace style for one-line if statements and loops. Provide an example and counter-example. Change-Id: Iafeea977203b76c0e67385779fb4ed57f3c6699a Reviewed-on: https://gerrit.openafs.org/12370 Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
300 lines
10 KiB
Plaintext
300 lines
10 KiB
Plaintext
Notes on Coding Standards/Requirements for OpenAFS Source
|
|
---------------------------------------------------------
|
|
|
|
We have an official style. Please use it. If you have gnu indent 2.2.9 or
|
|
later you can reformat for this style with the following option:
|
|
|
|
-npro -nbad -bap -nbc -bbo -br -ce -cdw -brs -ncdb -cp1 -ncs -di2 -ndj -nfc1
|
|
-nfca -i4 -lp -npcs -nprs -psl -sc -nsob -ts8
|
|
|
|
Do not use $< for non-pattern rules in any cross-platform dir as it
|
|
requires a reasonable make that is not available on all systems.
|
|
|
|
Do not have build rules that build multiple targets. Make doesn't seem able
|
|
to handle this, and it interferes with -j builds. (In particular, build the
|
|
rxgen targets individually and not using the flags for building all the files
|
|
in one shot.)
|
|
|
|
Try to test builds using gmake -j # MAKE="gmake -j #", it seems like a good
|
|
way to find missing or order-dependent dependency rules. (Is there a better
|
|
way to do this?)
|
|
|
|
-- Prototyping and Style --
|
|
Prototypes for all source files in a given dir DDD should be placed
|
|
in the file DDD/DDD_prototypes.h. All externally used (either API
|
|
or used by other source files) routines and variables should be
|
|
prototyped in this file.
|
|
|
|
The prototypes should be a full prototype, with argument and return
|
|
types. (Should not generate a warning with gcc -Wstrict-prototypes.)
|
|
|
|
Format of the prototype files should look like:
|
|
|
|
Standard Copyright Notice
|
|
|
|
#ifndef AFS_SRC_DDD_PROTO_H
|
|
#define AFS_SRC_DDD_PROTO_H
|
|
|
|
/* filename.c */
|
|
prototypes
|
|
|
|
/* filename.c */
|
|
prototypes
|
|
|
|
#endif /* AFS_SRC_DDD_PROTO_H */
|
|
|
|
In most of the existing prototypes, the define is DDD_PROTOTYPES_H, which is
|
|
probably ok as well.
|
|
|
|
The declaration of the routines should be done in ANSI style. If at some
|
|
later date, it is determined that prototypes don't work on some platform
|
|
properly, we can use ansi2knr during the compile.
|
|
|
|
rettype
|
|
routine(argtype arg)
|
|
{
|
|
|
|
}
|
|
|
|
All routines should have a return type specified, void if nothing returned,
|
|
and should have (void) if no arguments are taken.
|
|
|
|
Header files should not contain macros or other definitions unless they
|
|
are used across multiple source files.
|
|
|
|
All routines should be declared static if they are not used outside that
|
|
source file.
|
|
|
|
Compiles on gcc-using machines should strive to handle using
|
|
-Wstrict-prototypes -Werror. (this may take a while)
|
|
|
|
Routines shall be defined in source prior to use if possible, and
|
|
prototyped in block at top of file if static.
|
|
|
|
API documentation in the code should be done using Qt-style Doxygen
|
|
comments.
|
|
|
|
If you make a routine or variable static, be sure and remove it from
|
|
the AIX .exp files.
|
|
|
|
Suggested compiler flags:
|
|
gcc: -Wall -Wstrict-prototypes
|
|
Solaris Workshop CC: -fd -v
|
|
(You might not want the -fd, it isn't really useful, just complains about the
|
|
K&R style functions, but -v gives useful info.)
|
|
|
|
Multiple line comment blocks should begin with only /* on one line and end with
|
|
only */ on one line.
|
|
|
|
Example:
|
|
|
|
/*
|
|
* Multiple line comment blocks should be formatted
|
|
* like this example.
|
|
*/
|
|
|
|
Do not use braces on one-line if and loop statements.
|
|
|
|
Use:
|
|
|
|
if (some_condition)
|
|
do_some_action();
|
|
else
|
|
do_something_else();
|
|
|
|
while (some_condition)
|
|
do_something();
|
|
|
|
Instead of:
|
|
|
|
if (some_condition) {
|
|
do_some_action();
|
|
}
|
|
|
|
while (some_condition) {
|
|
do_something();
|
|
}
|
|
|
|
|
|
Dependencies required to build OpenAFS from source
|
|
--------------------------------------------------
|
|
The following packages are required to build all of the OpenAFS code
|
|
from source on various operating systems:
|
|
|
|
On Debian:
|
|
- autoconf, automake, bison, comerr-dev, cpio, flex, libkrb5-dev,
|
|
libncurses5-dev, libpam0g-dev, libxml2-utils, perl, pkg-config;
|
|
- libfuse-dev (for the FUSE-based user-mode client);
|
|
- dblatex, docbook-xsl, doxygen, xsltproc (for documentation);
|
|
- debhelper, hardening-wrapper, dkms (to build the Debian packages)
|
|
|
|
On FreeBSD:
|
|
- autoconf, automake, libtool;
|
|
- fusefs-libs, pkgconf (for the FUSE-based user-mode client);
|
|
- perl, dblatex, docbook-xsl, libxslt, python, ruby, zip (for documentation)
|
|
|
|
In addition, FreeBSD systems require kernel sources and a configured kernel
|
|
build directory (see section "FreeBSD Notes" in the README file).
|
|
|
|
GIT Usage
|
|
=========
|
|
|
|
*WARNING* *WARNING* *WARNING* *WARNING* *WARNING* *WARNING* *WARNING*
|
|
The Git tree may not always have code which can currently be built.
|
|
While every effort is made to keep the head of the tree buildable,
|
|
you may at any time find yourself between commits and hence have a tree
|
|
which does not build, or worse, causes more serious problems!
|
|
|
|
Do not use the Git tree unless you know what you're doing.
|
|
|
|
Git checkouts do not include files generated by autoconf. You can
|
|
run regen.sh (at the top level) to create these files. You will need
|
|
to have autoconf and automake installed on your system.
|
|
|
|
Summary
|
|
-------
|
|
|
|
Browse: http://git.openafs.org/
|
|
Clone: git clone git://git.openafs.org/openafs.git
|
|
|
|
Step-by-step
|
|
------------
|
|
|
|
1. Obtain the Git software. If you are using a system with a standard
|
|
software repository, Git may already be available as a package named
|
|
something like git or git-core. Otherwise, go to http://git-scm.com/
|
|
|
|
2. Run the command:
|
|
|
|
% git clone git://git.openafs.org/openafs.git
|
|
|
|
This will download the full repository and leave a checked-out tree in
|
|
a subdirectory of the current directory named openafs. The repository
|
|
itself is in the .git subdirectory of that directory.
|
|
|
|
WARNING: The repository is approximately 60MiB currently and will only
|
|
grow, so it may take some time to download the first time over a slow
|
|
network connection.
|
|
|
|
3. Generate the additional required files:
|
|
|
|
% cd openafs
|
|
% ./regen.sh
|
|
|
|
The current development series is in the branch named master. The stable
|
|
releases are on separate branches named something like
|
|
openafs-stable_<version> with a separate branch for each major stable
|
|
release series. Use git branch -a to see a full list of branches.
|
|
|
|
OpenAFS uses the Gerrit code review system to review and merge all changes
|
|
to OpenAFS. More details are at:
|
|
|
|
http://wiki.openafs.org/GitDevelopers/
|
|
|
|
including more detailed Git instructions.
|
|
|
|
It's by far preferred to use Gerrit to submit code changes, but if you
|
|
can't for whatever reason, you can instead open a bug and submit a patch
|
|
that way. Do this by sending mail to openafs-bugs@openafs.org with the
|
|
patch attached. But please use Gerrit if you can; patches sent in as bugs
|
|
will have to be forwarded to Gerrit by someone else, and it's easier for
|
|
everyone if you can enter them into Gerrit yourself.
|
|
|
|
Backport policy
|
|
------------
|
|
All patches should land on master first, unless the patch fixes a bug
|
|
that only exists in the stable branch.
|
|
|
|
Once a patch has been accepted into master, anyone can propose
|
|
backports to stable branches.
|
|
|
|
When cherry-picking a commit from another branch, please append a
|
|
"cherry picked from" section in your commit message. You'll also need
|
|
a separate Change-ID for Gerrit to recognize this as a separate
|
|
change. One workflow to do this:
|
|
|
|
1) Use "git cherry-pick -ex" to pick your commits onto another branch.
|
|
The -x option will append the appropriate "cherry picked from"
|
|
message, and the -e option will open your editor for you to edit
|
|
the commit message.
|
|
2) In your editor, delete the existing Change-ID line. Save and quit.
|
|
3) Run "git commit --amend", saving and quitting again. Git will run
|
|
the commit hook and generate a new Change-ID for Gerrit.
|
|
|
|
Warnings
|
|
========
|
|
|
|
OpenAFS Warning detection
|
|
-------------------------
|
|
|
|
There's been a concerted effort over the last few years, by many developers,
|
|
to reduce the number of warnings in the OpenAFS tree. In an attempt to
|
|
prevent warnings from creeping back in, we now have the ability to break the
|
|
build when new warnings appear.
|
|
|
|
This is only available for systems with gcc 4.2 or later or clang 3.2 or
|
|
later, and is disabled unless the --enable-checking option is supplied to
|
|
configure. Because we can't remove all of the warnings, we permit file by
|
|
file (and warning by warning) disabling of specific warnings. The
|
|
--enable-checking=all option prevents
|
|
this, and errors for any file containing a warning.
|
|
|
|
Disabling warnings
|
|
------------------
|
|
|
|
If warnings are unavoidable in a particular part of the build, they may be
|
|
disabled in an number of ways.
|
|
|
|
You can disable a single warning type in a particular file by using GCC
|
|
pragmas. If a warning can be disabled with a pragma, then the switch to use
|
|
will be listed in the error message you receive from the compiler. Pragmas
|
|
should be wrapped in IGNORE_SOME_GCC_WARNINGS, so that they aren't used
|
|
with non-gcc compilers, and can be disabled if desired. For example:
|
|
#ifdef IGNORE_SOME_GCC_WARNINGS
|
|
# pragma GCC diagnostic warning "-Wold-style-definition"
|
|
#endif
|
|
|
|
It would appear that when built with -Werror, the llvm clang compiler will
|
|
still upgrade warnings that are suppresed in this way to errors. In this case,
|
|
the fix is to mark that warning as ignored, but only for clang. For example:
|
|
#ifdef IGNORE_SOME_GCC_WARNINGS
|
|
# ifdef __clang__
|
|
# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
|
|
# else
|
|
# pragma GCC diagnostic warning "-Wdeprecated-declarations"
|
|
# endif
|
|
#endif
|
|
|
|
If a pragma isn't available for your particular warning, you will need to
|
|
disable all warnings for the file in question. You can do this by supplying
|
|
the autoconf macro @CFLAGS_NOERROR@ in the build options for the file. For
|
|
example:
|
|
lex.yy.o : lex.yy.c y.tab.c
|
|
${CC} -c ${CFLAGS} @CFLAGS_NOERROR@ lex.yy.c
|
|
|
|
If you add a new warning inhibition, please also add it to the list below.
|
|
|
|
Inhibited warnings
|
|
------------------
|
|
|
|
afs/afs_syscall.c : old-style
|
|
: strict-proto
|
|
: all (ukernel) : syscall pointer issues
|
|
afsd/afsd_kernel.c : deprecated : daemon() marked as deprecated on Darwin
|
|
auth/ktc.c : all (ukernel) : call_syscall doesn't have a prototype
|
|
bozo/bosserver.c : deprecated : daemon() marked as deprecated on Darwin
|
|
bucoord/ubik_db_if.c : strict-proto : Ubik_Call
|
|
bucoord/commands.c : all : Ubik_Call
|
|
: signed vs unsigned for dates
|
|
butc/tcudbprocs.c : all : ubik_Call
|
|
external/heimdal/hcrypto/validate.c: all: statement with empty body
|
|
kauth/admin_tools.c : strict-proto : ubik_Call
|
|
kauth/authclient.c : strict-proto : ubik_Call nonsense
|
|
libadmin/kas/afs_kasAdmin.c: strict-proto : ubik_Call nonsense
|
|
libadmin/samples/rxstat_query_peer.c : all : util_RPCStatsStateGet types
|
|
libadmin/samples/rxstat_query_process.c : all : util_RPCStatsStateGet types
|
|
libadmin/test/client.c : all : util_RPCStatsStateGet types
|
|
ubik/ubikclient.c : strict-protos : ubik_Call
|
|
volser/vol-dump.c : format : afs_sfsize_t
|
|
|