mirror of
https://github.com/Stichting-MINIX-Research-Foundation/netbsd.git
synced 2025-09-12 00:24:52 -04:00
454 lines
18 KiB
Plaintext
454 lines
18 KiB
Plaintext
How to write code for CVS
|
|
|
|
* License of CVS
|
|
|
|
CVS is Copyright (C) 1989-2005 The Free Software Foundation, Inc.
|
|
|
|
This program is free software; you can redistribute it and/or modify
|
|
it under the terms of the GNU General Public License as published by
|
|
the Free Software Foundation; either version 1, or (at your option)
|
|
any later version.
|
|
|
|
More details are available in the COPYING file but, in simplified
|
|
terms, this means that any distributed modifications you make to
|
|
this software must also be released under the GNU General Public
|
|
License.
|
|
|
|
This program is distributed in the hope that it will be useful,
|
|
but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
|
GNU General Public License for more details.
|
|
|
|
* Source
|
|
|
|
Patches against the development version of CVS are most likely to be accepted:
|
|
|
|
$ export CVS_RSH="ssh"
|
|
$ cvs -z3 -d:ext:anoncvs@savannah.nongnu.org:/cvsroot/cvs co ccvs
|
|
|
|
* Compiler options
|
|
|
|
If you are using GCC, you'll want to configure with -Wall, which can
|
|
detect many programming errors. This is not the default because it
|
|
might cause spurious warnings, but at least on some machines, there
|
|
should be no spurious warnings. For example:
|
|
|
|
$ ./configure CPPFLAGS=-Wall
|
|
|
|
* Backwards Compatibility
|
|
|
|
Only bug fixes are accepted into the stable branch. New features should be
|
|
applied to the trunk.
|
|
|
|
If it is not inextricable from a bug fix, CVS's output (to stdout/stderr)
|
|
should not be changed on the stable branch in order to best support scripts and
|
|
other tools which parse CVS's output. It is ok to change output between
|
|
feature releases (on the trunk), though such changes should be noted in the
|
|
NEWS file.
|
|
|
|
Changes in the way CVS responds to command line options, config options, etc.
|
|
should be accompanied by deprecation warnings for an entire stable series of
|
|
releases before being changed permanently, if at all possible.
|
|
|
|
* Indentation style
|
|
|
|
CVS mostly uses a consistent indentation style which looks like this:
|
|
|
|
void
|
|
foo (char *arg, int c)
|
|
{
|
|
long aflag;
|
|
|
|
if (arg)
|
|
{
|
|
bar (arg);
|
|
baz (arg);
|
|
}
|
|
|
|
switch (c)
|
|
{
|
|
case 'A':
|
|
aflag = 1;
|
|
break;
|
|
case 'E':
|
|
go to myerr;
|
|
}
|
|
|
|
printf ("Literal string line 1\n"
|
|
"Literal string line 2\n"
|
|
"Literal string line 3\n");
|
|
return;
|
|
|
|
myerr:
|
|
printf ("Error argument found\n");
|
|
}
|
|
|
|
- Do not cast NULL unless it is a stdarg argument to a function.
|
|
|
|
- Do not cast functions returning (void *), e.g., xmalloc ().
|
|
|
|
- Do not cast non-stdarg arguments to a function to '(void *)'
|
|
except to drop a 'const' modifier.
|
|
|
|
- Snuggle ! close to its expression (i.e., '! foo' => '!foo').
|
|
|
|
- Functions and C statements have a space before the "("
|
|
and the expression does not have a leading or trailing space
|
|
(i.e., 'if( foo )' => 'if (foo)'), although it is sometimes
|
|
desirable to add a newline after the "(" for #ifdef'd code.
|
|
|
|
- For switch statements, indent 'case' by 2 and the body of the case
|
|
by an additional 2 spaces.
|
|
|
|
- Labels should be indented by 2 spaces rather than the 4 spaces
|
|
used by the rest of the current block level.
|
|
|
|
|
|
while ((var = next_arg ()) != 0)
|
|
{
|
|
again:
|
|
switch (var)
|
|
{
|
|
case ONE:
|
|
code_for_case_one ();
|
|
break;
|
|
case TWO:
|
|
code_for_case_two ();
|
|
break;
|
|
case THREE:
|
|
push_arg (RESET_ONE);
|
|
var = ONE;
|
|
go to again;
|
|
default:
|
|
code_for_default_case ():
|
|
break;
|
|
}
|
|
}
|
|
|
|
- NULL-protected free goes on one line if possible, for example:
|
|
|
|
if (var)
|
|
free (var);
|
|
if (var2 != NULL)
|
|
free (var2);
|
|
|
|
should be written as:
|
|
|
|
if (var) free (var);
|
|
if (var2) free (var2);
|
|
|
|
if the value needs to be set to NULL after the free, then use
|
|
|
|
if (var)
|
|
{
|
|
free (var):
|
|
var = NULL;
|
|
}
|
|
|
|
as the idiom.
|
|
|
|
- Use whitespace in arithmetic expressions, for example
|
|
|
|
foo (arg+2);
|
|
|
|
should be written as
|
|
|
|
foo (arg + 2);
|
|
|
|
likewise for normal arithmetic expression assignments.
|
|
|
|
- Argument lists get a space after a comma.
|
|
|
|
- Do not parenthesize return values unless the expression needs to
|
|
span multiple lines.
|
|
|
|
- Cast negative constants when used in assignments or comparisons
|
|
with unsigned types.
|
|
|
|
- Try to be consistent with block comments:
|
|
|
|
/* This is a good block comment (spanning multiple lines of text).
|
|
* It starts with slash-star, leads each line with a star aligned with
|
|
* the first, and ends with a similarly aligned star-slash on a line
|
|
* by itself.
|
|
*/
|
|
|
|
/* This is a bad block comment,
|
|
because it can make it hard to tell what is code
|
|
and what is not code. */
|
|
|
|
- Sentences in comments should have a double space between each
|
|
period (.) and the beginning of the next sentence.
|
|
|
|
- Conditional expressions that need to be split should put the ?
|
|
operator on the new line.
|
|
|
|
- Follow GNU standards for breaking logical expressions over
|
|
multiple lines where possible.
|
|
|
|
- Do not snuggle open-lbrace blocks.
|
|
|
|
- Remove '#if 0' code where possible. Add a comment FIXME if it
|
|
really is a possible problem.
|
|
|
|
- Remove commented-out code where possible (FIXME blocks are
|
|
excepted).
|
|
|
|
The file cvs-format.el contains settings for emacs and the NEWS file
|
|
contains a set of options for the indent program which I haven't tried
|
|
but which are correct as far as I know. You will find some code which
|
|
does not conform to this indentation style; the plan is to re-indent it
|
|
as those sections of the code are changed (one function at a time,
|
|
perhaps).
|
|
|
|
In a submitted patch it is acceptable to refrain from changing the
|
|
indentation of large blocks of code to minimize the size of the patch;
|
|
the person checking in such a patch should re-indent it.
|
|
|
|
* Portability
|
|
|
|
The general rule for portability is that it is only worth including
|
|
portability cruft for systems on which people are actually testing and
|
|
using new CVS releases. Without testing, CVS will fail to be portable
|
|
for any number of unanticipated reasons.
|
|
|
|
CVS is now assuming a freestanding C89 compiler. If you don't have one, you
|
|
should find an old release of GCC that did not require a freestanding C89
|
|
compiler to build, build that on your system, build a newer release of GCC
|
|
if you wish, then build CVS using GCC as your freestanding C89 compiler.
|
|
|
|
A freestanding C89 compiler is guaranteed to support function prototypes,
|
|
void *, and assert().
|
|
|
|
The following headers can be assumed and are included from lib/system.h for a
|
|
freestanding C89 implementation: <float.h>, <limits.h>, <stdarg.h>, <stddef.h>.
|
|
We are not assuming the other standard headers listed by C89 (hosted headers)
|
|
because these four headers are the only headers guaranteed to be shipped with
|
|
a C89 compiler (freestanding compiler). We are not currently assuming that the
|
|
system the compiler is running on provides the rest of the C89 headers.
|
|
|
|
The following C89 hosted headers can be assumed due to their presence in UNIX
|
|
version 7 and are included from lib/system.h: <assert.h>, <ctype.h>, <errno.h>,
|
|
<math.h>, <setjmp.h>, <signal.h>, <stdio.h>. <time.h> can also be assumed but
|
|
is included via lib/xtime.h via lib/system.h to include some Autoconf magic
|
|
which avoids including <time.h> and <sys/time.h> on systems that can't handle
|
|
both.
|
|
|
|
The following C89 headers are also assumed since we believe GCC includes them
|
|
even on systems where it is installed as a freestanding compiler when the
|
|
system lacks them, despite their not being required: <stdlib.h>, <string.h>.
|
|
When the system does not lack these headers, they can sometimes not be
|
|
standards compatible, but GCC provides a script, `fixincludes', for the purpose
|
|
of fixing ANSI conformance problems and we think we can rely on asking users to
|
|
either use GCC or run this script to fix conformance problems manually. A
|
|
GNULIB developer has made a statement that if this turns out to be a problem,
|
|
GNULIB <stdlib.h> and <string.h> substitutes could be included in GNULIB, so if
|
|
we discover the problem, this should be discussed on <bug-gnulib@gnu.org>.
|
|
|
|
A substitute C99 <stdbool.h> is included from GNULIB for platforms that lack
|
|
this header. Please see the comments in the lib/stdbool_.h file for its
|
|
limitations.
|
|
|
|
<sys/types.h> can be assumed despite a lack of a presence in even C99, since
|
|
it has been around nearly forever and no-one has ever complained about our code
|
|
assuming its existence.
|
|
|
|
CVS has also been assuming <pwd.h> for some time. I am unsure of the
|
|
rationale.
|
|
|
|
GNULIB also assumes <sys/stat.h>. I am unsure of the rationale.
|
|
|
|
A substitute POSIX.2 <fnmatch.h> header and fnmatch() function is provided for
|
|
systems that lack them. Similarly for the non-standard <alloca.h> header and
|
|
alloca() function. Other substitute headers and functions are also provided
|
|
when needed. See the lib directory or the maint-aux/srclist.txt file for more
|
|
information.
|
|
|
|
Please do not use multi-line strings. Despite their common acceptance by many
|
|
compilers, they are not part of the ANSI C specification. As of GCC version
|
|
3.3, they are no longer supported. See the Indentation Style section above for
|
|
an example of a literal string which is not multi-line but which will print
|
|
multiple lines.
|
|
|
|
* Other style issues
|
|
|
|
When composing header files, do not declare function prototypes using the
|
|
`extern' storage-class identifier. Under C89, there is no functional
|
|
difference between a function declaration with and without `extern', unless the
|
|
function is declared `static'. This is NOT the case for global variables.
|
|
Global variables declared in header files MUST be declared `extern'. For
|
|
example:
|
|
|
|
/* Global variables */
|
|
extern int foo;
|
|
extern char *bar;
|
|
|
|
/* Function declarations */
|
|
int make_foo(void);
|
|
char *make_bar(int _foobar);
|
|
|
|
* Run-time behaviors
|
|
|
|
Use assert() to check "can't happen" conditions internal to CVS. We
|
|
realize that there are functions in CVS which instead return NULL or
|
|
some such value (thus confusing the meaning of such a returned value),
|
|
but we want to fix that code. Of course, bad input data, a corrupt
|
|
repository, bad options, etc., should always print a real error
|
|
message instead.
|
|
|
|
Do not use arbitrary limits (such as PATH_MAX) except perhaps when the
|
|
operating system or some external interface requires it. We spent a
|
|
lot of time getting rid of them, and we don't want to put them back.
|
|
If you find any that we missed, please report it as with other bugs.
|
|
In most cases such code will create security holes (for example, for
|
|
anonymous read-only access via the CVS protocol, or if a WWW cgi script
|
|
passes client-supplied arguments to CVS).
|
|
|
|
Although this is a long-term goal, it also would be nice to move CVS
|
|
in the direction of reentrancy. This reduces the size of the data
|
|
segment and will allow a multi-threaded server if that is desirable.
|
|
It is also useful to write the code so that it can be easily be made
|
|
reentrant later. For example, if you need to pass data to some functions,
|
|
you need a static variable, but use a single pointer so that when the function
|
|
is fixed to pass along the argument, then the code can easily use that
|
|
argument.
|
|
|
|
* Coding standards in general
|
|
|
|
Generally speaking the GNU coding standards are mostly used by CVS
|
|
(but see the exceptions mentioned above, such as indentation style,
|
|
and perhaps an exception or two we haven't mentioned). This is the
|
|
file standards.text at the GNU FTP sites. The primary URL for this
|
|
information is http://www.gnu.org/prep/standards/ which contains links
|
|
to many different formats of the standards.
|
|
|
|
* Regenerating Build Files (UNIX)
|
|
|
|
On UNIX, if you wish to change the build files, you will need Autoconf and
|
|
Automake.
|
|
|
|
Some combinations of Automake and Autoconf versions may break the
|
|
CVS build if file timestamps aren't set correctly and people don't
|
|
have the same versions the developers do, so the rules to run them
|
|
automatically aren't included in the generated Makefiles unless you run
|
|
configure with --enable-maintainer-mode.
|
|
|
|
The CVS Makefiles and configure script were built using Automake 1.9.6 and
|
|
Autoconf 2.59, respectively.
|
|
|
|
There is a known bug in Autoconf 2.57 that will prevent the configure
|
|
scripts it generates from working on some platforms. Other combinations of
|
|
autotool versions may or may not work. If you get other versions to work,
|
|
please send a report to <bug-cvs@nongnu.org>.
|
|
|
|
* Regenerating Build Files (Windows)
|
|
|
|
If for some reason you end up regenerating the *.mak files to submit a patch,
|
|
please run windows-NT/fix-msvc-mak.pl to remove the absolute paths from the
|
|
generated *.mak files before generating any patches.
|
|
|
|
* Rebuilding Yacc sources
|
|
|
|
The lib/getdate.y file requires GNU Bison 1.875 to rebuild lib/getdate.c. Not
|
|
having GNU Bison 1.875 should not stop the build unless the lib/getdate.c file
|
|
is actually missing, perhaps deleted via `make maintainerclean'.
|
|
|
|
* Writing patches (strategy)
|
|
|
|
Only some kinds of changes are suitable for inclusion in the
|
|
"official" CVS. Bugfixes, where CVS's behavior contradicts the
|
|
documentation and/or expectations that everyone agrees on, should be
|
|
OK (strategically). For features, the desirable attributes are that
|
|
the need is clear and that they fit nicely into the architecture of
|
|
CVS. Is it worth the cost (in terms of complexity or any other
|
|
tradeoffs involved)? Are there better solutions?
|
|
|
|
If the design is not yet clear (which is true of most features), then
|
|
the design is likely to benefit from more work and community input.
|
|
Make a list of issues, or write documentation including rationales for
|
|
how one would use the feature. Discuss it with coworkers, a
|
|
newsgroup, or a mailing list, and see what other people think.
|
|
Distribute some experimental patches and see what people think. The
|
|
intention is arrive at some kind of rough community consensus before
|
|
changing the "official" CVS. Features like zlib, encryption, and
|
|
the RCS library have benefited from this process in the past.
|
|
|
|
If longstanding CVS behavior, that people may be relying on, is
|
|
clearly deficient, it can be changed, but only slowly and carefully.
|
|
For example, the global -q option was introduced in CVS 1.3 but the
|
|
command -q options, which the global -q replaced, were not removed
|
|
until CVS 1.6.
|
|
|
|
* Writing patches (tactics)
|
|
|
|
When you first distribute a patch it may be suitable to just put forth
|
|
a rough patch, or even just an idea. But before the end of the
|
|
process the following should exist:
|
|
|
|
- ChangeLog entry (see the GNU coding standards for details).
|
|
|
|
- Changes to the NEWS file and cvs.texinfo, if the change is a
|
|
user-visible change worth mentioning.
|
|
|
|
- Somewhere, a description of what the patch fixes (often in
|
|
comments in the code, or maybe the ChangeLog or documentation).
|
|
|
|
- Most of the time, a test case (see TESTS). It can be quite
|
|
frustrating to fix a bug only to see it reappear later, and adding
|
|
the case to the testsuite, where feasible, solves this and other
|
|
problems. See the TESTS file for notes on writing new tests.
|
|
|
|
If you solve several unrelated problems, it is generally easier to
|
|
consider the desirability of the changes if there is a separate patch
|
|
for each issue. Use context diffs or unidiffs for patches.
|
|
|
|
Include words like "I grant permission to distribute this patch under
|
|
the terms of the GNU Public License" with your patch. By sending a
|
|
patch to bug-cvs@nongnu.org, you implicitly grant this permission.
|
|
|
|
Submitting a patch to bug-cvs is the way to reach the people who have
|
|
signed up to receive such submissions (including CVS developers), but
|
|
there may or may not be much (or any) response. If you want to pursue
|
|
the matter further, you are probably best off working with the larger
|
|
CVS community. Distribute your patch as widely as desired (mailing
|
|
lists, newsgroups, web sites, whatever). Write a web page or other
|
|
information describing what the patch is for. It is neither practical
|
|
nor desirable for all/most contributions to be distributed through the
|
|
"official" (whatever that means) mechanisms of CVS releases and CVS
|
|
developers. Now, the "official" mechanisms do try to incorporate
|
|
those patches which seem most suitable for widespread usage, together
|
|
with test cases and documentation. So if a patch becomes sufficiently
|
|
popular in the CVS community, it is likely that one of the CVS
|
|
developers will eventually try to do something with it. But dealing
|
|
with the CVS developers may be the last step of the process rather
|
|
than the first.
|
|
|
|
* What is the schedule for the next release?
|
|
|
|
There isn't one. That is, upcoming releases are not announced (or
|
|
even hinted at, really) until the feature freeze which is
|
|
approximately 2 weeks before the final release (at this time test
|
|
releases start appearing and are announced on info-cvs). This is
|
|
intentional, to avoid a last minute rush to get new features in.
|
|
|
|
* Mailing lists
|
|
|
|
In addition to the mailing lists listed in the README file, developers should
|
|
take particular note of the following mailling lists:
|
|
|
|
bug-cvs: This is the list which users are requested to send bug reports
|
|
to. General CVS development and design discussions also take place on
|
|
this list.
|
|
info-cvs: This list is intended for user questions, but general CVS
|
|
development and design discussions sometimes take place on this list.
|
|
cvs-cvs: The only messages sent to this list are sent
|
|
automatically, via the CVS `loginfo' mechanism, when someone
|
|
checks something in to the master CVS repository.
|
|
cvs-test-results: The only messages sent to this list are sent
|
|
automatically, daily, by a script which runs "make check"
|
|
and "make remotecheck" on the master CVS sources.
|
|
|
|
To subscribe to any of these lists, send mail to <list>-request@nongnu.org
|
|
or visit http://savannah.nongnu.org/mail/?group=cvs and follow the instructions
|
|
for the list you wish to subscribe to.
|