mirror of
https://git.openafs.org/openafs.git
synced 2025-01-18 15:00:12 +00:00
f9c96d0fd6
Every function should be explicitly declared before it can be called. Since -Wimplicit-function-declaration is usually a warning and not an error, calling undeclared functions does not prevent us from building the code. However, apple-clang 12 makes this an error by default, prohibiting the build of the current version on macOS 11 (Big Sur). To fix this problem, declare functions before calling them. Also, include mach/thread_act.h into afs_call.c so the declaration of thread_terminate() can be found on macOS. Last, given that the third argument of PIOCTL() (if UKERNEL is defined) is a pointer, cast it to 'long'. Doing so, we can avoid another inhibited warning. Notice that this PIOCTL definition is scoped to a single file (src/auth/ktc.c). Change-Id: I6d796c10ea4dd81b13ae5feb9f42608aa445560d Reviewed-on: https://gerrit.openafs.org/14744 Reviewed-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: Cheyenne Wills <cwills@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com>
354 lines
12 KiB
Plaintext
354 lines
12 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();
|
|
}
|
|
|
|
In switch statements, to fall through from one case statement to another, use
|
|
AFS_FALLTHROUGH to mark the intentional fall through. Do not use fall through
|
|
comments (e.g. /* fallthrough */), as some compilers do not recognize them and
|
|
will flag the case statement with an implied fallthrough warning.
|
|
|
|
Use:
|
|
|
|
switch (x) {
|
|
case 1:
|
|
do_something();
|
|
AFS_FALLTHROUGH;
|
|
case 2:
|
|
do_something_else();
|
|
AFS_FALLTHROUGH;
|
|
default:
|
|
do_some_action();
|
|
}
|
|
|
|
Instead of using fallthrough comments:
|
|
|
|
switch (x) {
|
|
case 1:
|
|
do_something();
|
|
/* fallthrough */
|
|
case 2:
|
|
do_something_else();
|
|
/* fallthrough */
|
|
default:
|
|
do_some_action();
|
|
}
|
|
|
|
Or not marking the fall through:
|
|
|
|
switch (x) {
|
|
case 1:
|
|
do_something();
|
|
case 2:
|
|
do_something_else();
|
|
default:
|
|
do_some_action();
|
|
}
|
|
|
|
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 the source cannot be changed to add a pragma, you might be abe to use the
|
|
autoconf function AX_APPEND_COMPILE_FLAGS to create a new macro that disables
|
|
the warning and then use macro for the build options for that file. For an
|
|
example, see how the autoconf macro CFLAGS_NOIMPLICIT_FALLTHROUGH is defined and
|
|
used.
|
|
|
|
Finally if there isn't a way to disable the specific 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
|
|
------------------
|
|
uss/lex.i : fallthrough : clang fallthrough, flex generated code
|
|
comerr/et_lex.lex.l : fallthrough : clang fallthrough, flex generated code
|
|
pragma set to ignored where included in
|
|
error_table.y
|
|
afs/afs_syscall.c : old-style
|
|
: strict-proto
|
|
: all (ukernel) : syscall pointer issues
|
|
afsd/afsd_kernel.c : deprecated : daemon() marked as deprecated on Darwin
|
|
bozo/bosserver.c : deprecated : daemon() marked as deprecated on Darwin
|
|
bucoord/ubik_db_if.c : strict-proto : ubik_Call_SingleServer
|
|
bucoord/commands.c : all : signed vs unsigned for dates
|
|
external/heimdal/hcrypto/validate.c: all: statement with empty body
|
|
external/heimdal/hcrypto/evp.c: cast-function-type
|
|
: Linux kernel build uses -Wcast-function-type
|
|
external/heimdal/hcrypto/evp-algs.c: cast-function-type
|
|
: Linux kernel build uses -Wcast-function-type
|
|
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
|
|
rxkad/ticket5.c : format-truncation : inside included file v5der.c in the
|
|
function _heim_time2generalizedtime, the
|
|
two snprintf calls raise
|
|
format-truncation warnings due to the
|
|
arithmetic on tm_year and tm_mon fields |