diff options
Diffstat (limited to 'HACKING')
| -rw-r--r-- | HACKING | 453 |
1 files changed, 453 insertions, 0 deletions
@@ -0,0 +1,453 @@ +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. |