1How to write code for CVS 2 3* License of CVS 4 5 CVS is Copyright (C) 1989-2005 The Free Software Foundation, Inc. 6 7 This program is free software; you can redistribute it and/or modify 8 it under the terms of the GNU General Public License as published by 9 the Free Software Foundation; either version 1, or (at your option) 10 any later version. 11 12 More details are available in the COPYING file but, in simplified 13 terms, this means that any distributed modifications you make to 14 this software must also be released under the GNU General Public 15 License. 16 17 This program is distributed in the hope that it will be useful, 18 but WITHOUT ANY WARRANTY; without even the implied warranty of 19 MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the 20 GNU General Public License for more details. 21 22* Source 23 24Patches against the development version of CVS are most likely to be accepted: 25 26 $ export CVS_RSH="ssh" 27 $ cvs -z3 -d:ext:anoncvs@savannah.nongnu.org:/cvsroot/cvs co ccvs 28 29* Compiler options 30 31If you are using GCC, you'll want to configure with -Wall, which can 32detect many programming errors. This is not the default because it 33might cause spurious warnings, but at least on some machines, there 34should be no spurious warnings. For example: 35 36 $ ./configure CPPFLAGS=-Wall 37 38* Backwards Compatibility 39 40Only bug fixes are accepted into the stable branch. New features should be 41applied to the trunk. 42 43If it is not inextricable from a bug fix, CVS's output (to stdout/stderr) 44should not be changed on the stable branch in order to best support scripts and 45other tools which parse CVS's output. It is ok to change output between 46feature releases (on the trunk), though such changes should be noted in the 47NEWS file. 48 49Changes in the way CVS responds to command line options, config options, etc. 50should be accompanied by deprecation warnings for an entire stable series of 51releases before being changed permanently, if at all possible. 52 53* Indentation style 54 55CVS mostly uses a consistent indentation style which looks like this: 56 57void 58foo (char *arg, int c) 59{ 60 long aflag; 61 62 if (arg) 63 { 64 bar (arg); 65 baz (arg); 66 } 67 68 switch (c) 69 { 70 case 'A': 71 aflag = 1; 72 break; 73 case 'E': 74 go to myerr; 75 } 76 77 printf ("Literal string line 1\n" 78 "Literal string line 2\n" 79 "Literal string line 3\n"); 80 return; 81 82 myerr: 83 printf ("Error argument found\n"); 84} 85 86 - Do not cast NULL unless it is a stdarg argument to a function. 87 88 - Do not cast functions returning (void *), e.g., xmalloc (). 89 90 - Do not cast non-stdarg arguments to a function to '(void *)' 91 except to drop a 'const' modifier. 92 93 - Snuggle ! close to its expression (i.e., '! foo' => '!foo'). 94 95 - Functions and C statements have a space before the "(" 96 and the expression does not have a leading or trailing space 97 (i.e., 'if( foo )' => 'if (foo)'), although it is sometimes 98 desirable to add a newline after the "(" for #ifdef'd code. 99 100 - For switch statements, indent 'case' by 2 and the body of the case 101 by an additional 2 spaces. 102 103 - Labels should be indented by 2 spaces rather than the 4 spaces 104 used by the rest of the current block level. 105 106 107 while ((var = next_arg ()) != 0) 108 { 109 again: 110 switch (var) 111 { 112 case ONE: 113 code_for_case_one (); 114 break; 115 case TWO: 116 code_for_case_two (); 117 break; 118 case THREE: 119 push_arg (RESET_ONE); 120 var = ONE; 121 go to again; 122 default: 123 code_for_default_case (): 124 break; 125 } 126 } 127 128 - NULL-protected free goes on one line if possible, for example: 129 130 if (var) 131 free (var); 132 if (var2 != NULL) 133 free (var2); 134 135 should be written as: 136 137 if (var) free (var); 138 if (var2) free (var2); 139 140 if the value needs to be set to NULL after the free, then use 141 142 if (var) 143 { 144 free (var): 145 var = NULL; 146 } 147 148 as the idiom. 149 150 - Use whitespace in arithmetic expressions, for example 151 152 foo (arg+2); 153 154 should be written as 155 156 foo (arg + 2); 157 158 likewise for normal arithmetic expression assignments. 159 160 - Argument lists get a space after a comma. 161 162 - Do not parenthesize return values unless the expression needs to 163 span multiple lines. 164 165 - Cast negative constants when used in assignments or comparisons 166 with unsigned types. 167 168 - Try to be consistent with block comments: 169 170 /* This is a good block comment (spanning multiple lines of text). 171 * It starts with slash-star, leads each line with a star aligned with 172 * the first, and ends with a similarly aligned star-slash on a line 173 * by itself. 174 */ 175 176 /* This is a bad block comment, 177 because it can make it hard to tell what is code 178 and what is not code. */ 179 180 - Sentences in comments should have a double space between each 181 period (.) and the beginning of the next sentence. 182 183 - Conditional expressions that need to be split should put the ? 184 operator on the new line. 185 186 - Follow GNU standards for breaking logical expressions over 187 multiple lines where possible. 188 189 - Do not snuggle open-lbrace blocks. 190 191 - Remove '#if 0' code where possible. Add a comment FIXME if it 192 really is a possible problem. 193 194 - Remove commented-out code where possible (FIXME blocks are 195 excepted). 196 197The file cvs-format.el contains settings for emacs and the NEWS file 198contains a set of options for the indent program which I haven't tried 199but which are correct as far as I know. You will find some code which 200does not conform to this indentation style; the plan is to re-indent it 201as those sections of the code are changed (one function at a time, 202perhaps). 203 204In a submitted patch it is acceptable to refrain from changing the 205indentation of large blocks of code to minimize the size of the patch; 206the person checking in such a patch should re-indent it. 207 208* Portability 209 210The general rule for portability is that it is only worth including 211portability cruft for systems on which people are actually testing and 212using new CVS releases. Without testing, CVS will fail to be portable 213for any number of unanticipated reasons. 214 215CVS is now assuming a freestanding C89 compiler. If you don't have one, you 216should find an old release of GCC that did not require a freestanding C89 217compiler to build, build that on your system, build a newer release of GCC 218if you wish, then build CVS using GCC as your freestanding C89 compiler. 219 220A freestanding C89 compiler is guaranteed to support function prototypes, 221void *, and assert(). 222 223The following headers can be assumed and are included from lib/system.h for a 224freestanding C89 implementation: <float.h>, <limits.h>, <stdarg.h>, <stddef.h>. 225We are not assuming the other standard headers listed by C89 (hosted headers) 226because these four headers are the only headers guaranteed to be shipped with 227a C89 compiler (freestanding compiler). We are not currently assuming that the 228system the compiler is running on provides the rest of the C89 headers. 229 230The following C89 hosted headers can be assumed due to their presence in UNIX 231version 7 and are included from lib/system.h: <assert.h>, <ctype.h>, <errno.h>, 232<math.h>, <setjmp.h>, <signal.h>, <stdio.h>. <time.h> can also be assumed but 233is included via lib/xtime.h via lib/system.h to include some Autoconf magic 234which avoids including <time.h> and <sys/time.h> on systems that can't handle 235both. 236 237The following C89 headers are also assumed since we believe GCC includes them 238even on systems where it is installed as a freestanding compiler when the 239system lacks them, despite their not being required: <stdlib.h>, <string.h>. 240When the system does not lack these headers, they can sometimes not be 241standards compatible, but GCC provides a script, `fixincludes', for the purpose 242of fixing ANSI conformance problems and we think we can rely on asking users to 243either use GCC or run this script to fix conformance problems manually. A 244GNULIB developer has made a statement that if this turns out to be a problem, 245GNULIB <stdlib.h> and <string.h> substitutes could be included in GNULIB, so if 246we discover the problem, this should be discussed on <bug-gnulib@gnu.org>. 247 248A substitute C99 <stdbool.h> is included from GNULIB for platforms that lack 249this header. Please see the comments in the lib/stdbool_.h file for its 250limitations. 251 252<sys/types.h> can be assumed despite a lack of a presence in even C99, since 253it has been around nearly forever and no-one has ever complained about our code 254assuming its existence. 255 256CVS has also been assuming <pwd.h> for some time. I am unsure of the 257rationale. 258 259GNULIB also assumes <sys/stat.h>. I am unsure of the rationale. 260 261A substitute POSIX.2 <fnmatch.h> header and fnmatch() function is provided for 262systems that lack them. Similarly for the non-standard <alloca.h> header and 263alloca() function. Other substitute headers and functions are also provided 264when needed. See the lib directory or the maint-aux/srclist.txt file for more 265information. 266 267Please do not use multi-line strings. Despite their common acceptance by many 268compilers, they are not part of the ANSI C specification. As of GCC version 2693.3, they are no longer supported. See the Indentation Style section above for 270an example of a literal string which is not multi-line but which will print 271multiple lines. 272 273* Other style issues 274 275When composing header files, do not declare function prototypes using the 276`extern' storage-class identifier. Under C89, there is no functional 277difference between a function declaration with and without `extern', unless the 278function is declared `static'. This is NOT the case for global variables. 279Global variables declared in header files MUST be declared `extern'. For 280example: 281 282/* Global variables */ 283extern int foo; 284extern char *bar; 285 286/* Function declarations */ 287int make_foo(void); 288char *make_bar(int _foobar); 289 290* Run-time behaviors 291 292Use assert() to check "can't happen" conditions internal to CVS. We 293realize that there are functions in CVS which instead return NULL or 294some such value (thus confusing the meaning of such a returned value), 295but we want to fix that code. Of course, bad input data, a corrupt 296repository, bad options, etc., should always print a real error 297message instead. 298 299Do not use arbitrary limits (such as PATH_MAX) except perhaps when the 300operating system or some external interface requires it. We spent a 301lot of time getting rid of them, and we don't want to put them back. 302If you find any that we missed, please report it as with other bugs. 303In most cases such code will create security holes (for example, for 304anonymous read-only access via the CVS protocol, or if a WWW cgi script 305passes client-supplied arguments to CVS). 306 307Although this is a long-term goal, it also would be nice to move CVS 308in the direction of reentrancy. This reduces the size of the data 309segment and will allow a multi-threaded server if that is desirable. 310It is also useful to write the code so that it can be easily be made 311reentrant later. For example, if you need to pass data to some functions, 312you need a static variable, but use a single pointer so that when the function 313is fixed to pass along the argument, then the code can easily use that 314argument. 315 316* Coding standards in general 317 318Generally speaking the GNU coding standards are mostly used by CVS 319(but see the exceptions mentioned above, such as indentation style, 320and perhaps an exception or two we haven't mentioned). This is the 321file standards.text at the GNU FTP sites. The primary URL for this 322information is http://www.gnu.org/prep/standards/ which contains links 323to many different formats of the standards. 324 325* Regenerating Build Files (UNIX) 326 327On UNIX, if you wish to change the build files, you will need Autoconf and 328Automake. 329 330Some combinations of Automake and Autoconf versions may break the 331CVS build if file timestamps aren't set correctly and people don't 332have the same versions the developers do, so the rules to run them 333automatically aren't included in the generated Makefiles unless you run 334configure with --enable-maintainer-mode. 335 336The CVS Makefiles and configure script were built using Automake 1.9.6 and 337Autoconf 2.59, respectively. 338 339There is a known bug in Autoconf 2.57 that will prevent the configure 340scripts it generates from working on some platforms. Other combinations of 341autotool versions may or may not work. If you get other versions to work, 342please send a report to <bug-cvs@nongnu.org>. 343 344* Regenerating Build Files (Windows) 345 346If for some reason you end up regenerating the *.mak files to submit a patch, 347please run windows-NT/fix-msvc-mak.pl to remove the absolute paths from the 348generated *.mak files before generating any patches. 349 350* Rebuilding Yacc sources 351 352The lib/getdate.y file requires GNU Bison 1.875 to rebuild lib/getdate.c. Not 353having GNU Bison 1.875 should not stop the build unless the lib/getdate.c file 354is actually missing, perhaps deleted via `make maintainerclean'. 355 356* Writing patches (strategy) 357 358Only some kinds of changes are suitable for inclusion in the 359"official" CVS. Bugfixes, where CVS's behavior contradicts the 360documentation and/or expectations that everyone agrees on, should be 361OK (strategically). For features, the desirable attributes are that 362the need is clear and that they fit nicely into the architecture of 363CVS. Is it worth the cost (in terms of complexity or any other 364tradeoffs involved)? Are there better solutions? 365 366If the design is not yet clear (which is true of most features), then 367the design is likely to benefit from more work and community input. 368Make a list of issues, or write documentation including rationales for 369how one would use the feature. Discuss it with coworkers, a 370newsgroup, or a mailing list, and see what other people think. 371Distribute some experimental patches and see what people think. The 372intention is arrive at some kind of rough community consensus before 373changing the "official" CVS. Features like zlib, encryption, and 374the RCS library have benefited from this process in the past. 375 376If longstanding CVS behavior, that people may be relying on, is 377clearly deficient, it can be changed, but only slowly and carefully. 378For example, the global -q option was introduced in CVS 1.3 but the 379command -q options, which the global -q replaced, were not removed 380until CVS 1.6. 381 382* Writing patches (tactics) 383 384When you first distribute a patch it may be suitable to just put forth 385a rough patch, or even just an idea. But before the end of the 386process the following should exist: 387 388 - ChangeLog entry (see the GNU coding standards for details). 389 390 - Changes to the NEWS file and cvs.texinfo, if the change is a 391 user-visible change worth mentioning. 392 393 - Somewhere, a description of what the patch fixes (often in 394 comments in the code, or maybe the ChangeLog or documentation). 395 396 - Most of the time, a test case (see TESTS). It can be quite 397 frustrating to fix a bug only to see it reappear later, and adding 398 the case to the testsuite, where feasible, solves this and other 399 problems. See the TESTS file for notes on writing new tests. 400 401If you solve several unrelated problems, it is generally easier to 402consider the desirability of the changes if there is a separate patch 403for each issue. Use context diffs or unidiffs for patches. 404 405Include words like "I grant permission to distribute this patch under 406the terms of the GNU Public License" with your patch. By sending a 407patch to bug-cvs@nongnu.org, you implicitly grant this permission. 408 409Submitting a patch to bug-cvs is the way to reach the people who have 410signed up to receive such submissions (including CVS developers), but 411there may or may not be much (or any) response. If you want to pursue 412the matter further, you are probably best off working with the larger 413CVS community. Distribute your patch as widely as desired (mailing 414lists, newsgroups, web sites, whatever). Write a web page or other 415information describing what the patch is for. It is neither practical 416nor desirable for all/most contributions to be distributed through the 417"official" (whatever that means) mechanisms of CVS releases and CVS 418developers. Now, the "official" mechanisms do try to incorporate 419those patches which seem most suitable for widespread usage, together 420with test cases and documentation. So if a patch becomes sufficiently 421popular in the CVS community, it is likely that one of the CVS 422developers will eventually try to do something with it. But dealing 423with the CVS developers may be the last step of the process rather 424than the first. 425 426* What is the schedule for the next release? 427 428There isn't one. That is, upcoming releases are not announced (or 429even hinted at, really) until the feature freeze which is 430approximately 2 weeks before the final release (at this time test 431releases start appearing and are announced on info-cvs). This is 432intentional, to avoid a last minute rush to get new features in. 433 434* Mailing lists 435 436In addition to the mailing lists listed in the README file, developers should 437take particular note of the following mailling lists: 438 439 bug-cvs: This is the list which users are requested to send bug reports 440 to. General CVS development and design discussions also take place on 441 this list. 442 info-cvs: This list is intended for user questions, but general CVS 443 development and design discussions sometimes take place on this list. 444 cvs-cvs: The only messages sent to this list are sent 445 automatically, via the CVS `loginfo' mechanism, when someone 446 checks something in to the master CVS repository. 447 cvs-test-results: The only messages sent to this list are sent 448 automatically, daily, by a script which runs "make check" 449 and "make remotecheck" on the master CVS sources. 450 451To subscribe to any of these lists, send mail to <list>-request@nongnu.org 452or visit http://savannah.nongnu.org/mail/?group=cvs and follow the instructions 453for the list you wish to subscribe to. 454