diff --git a/doc/coding-guidelines b/doc/coding-guidelines index 521bcd1ca..b40b161a5 100644 --- a/doc/coding-guidelines +++ b/doc/coding-guidelines @@ -1,143 +1,132 @@ -Rules about patches & modifications to UnrealIRCd - -1. When making a change, always add a small description in the commit log. - Don't forget to mention the bug# and credit the reporter (if any). - -2. If new files are made, it must contain proper copyright headers. - -3. If you want to submit patches (f.e. if you don't have write access to - the repository), then submit them to https://bugs.unrealircd.org/ - using "hg export" or "hg diff". Naturally include a clear description - of what the change does. - -4. Each bug or feature should have a bug# so people can have a discussion - about it. This has a few implications (read!!): - * People must report bugs/feature requests to bugs.unrealircd.org and - not on IRC, e-mail, etc. - * That means other people can see the bug# and comment on it. This means - discussion is easy to read back for each issue and not spread between - several IRC logs. - Furthermore, by using the bugtracker instead of directly committing, - people could point out that there might be a better way to do things - than you originally thought, or it might be that other devs don't like - it at all. - * If a head coder has 'acknowledged' or 'confirmed' the bug or stated in - a comment that it's OK to implement, then a dev may take the issue. - The dev should change the status to 'assigned' and work on it, then - commit and change it to 'resolved', set 'fixed in version' to next - release, and add a comment pasting the relevant Changelog item and the - releaseid (.XYZ). - Of course other guidelines, like #7 and #8 still apply. - -5. Do not commit changes that do not have an associated bug# and have not - had any discussion. - 3.2.x: Small/tiny bugfixes that do not change any functionality, are - very unlikely to break anything and definitely don't require any prior - discussion may be exempted. - 3.4.x: During the alpha & beta stage it is permitted to commit fixes - and code cleanups / restructuring without any discussion. - However in general, and in particular for new features, it is appreciated - if there has been prior discussion on bugs.unrealircd.org (or by mail). - -6. Regarding reidenting, restructuring or other major code cleanups: please - discuss before doing so. The other devs might not agree with you on the - particular cleanup you have in mind which would result in another - clean-up-the-cleanup commit. - You may, however reindent and clean up individual sections when you are - working on fixing a particular bug# or implementing a new feature. In fact - you're encouraged to do so if the code is confusing without it. However, - obey the style of Unreal's code (mostly outlined in this document) - and do not introduce yet another (new) style. Also, be careful with doing - any cleanup: if you're unsure in any way about the use of something, - or something that looks redundant on first sight, then look more - carefully... it might indeed be useless and/or redundant, but it might - also be a subtle thing that can create great bugs when 'cleaned up'. - -7. Prior to a 3.2.x release: be very careful with any restructuring of a - subsystem or doing any major commits that may break things. Stuff like - this can be perfectly fine if there are many months to go, but are not - good to do a month before release. The head coder may impose additional - restrictions during such a period. - -8. During the Release Candidate stage (from RC1 until the final release) - only the head coder may commit directly, all others should ask and - present their patch before committing. Yes, even if you are changing - only 1 line of code or text. - -9. UnrealIRCd should compile on all supported operating systems and - platforms, using GCC 3 or higher on *NIX, and Visual Studio 2008 or - higher on Windows. This means you cannot blindly use all C99 extensions. - -10. Coders must test their code before committing. - -11. /* - * These kind of comments - */ - - NOT - - // These kind of comments - -12. if (something == 1) - { - moo; /* comment */ - /* This does what what what */ - cow(go(moo)); - } - - NOT - - if (something == 1) { - } - -13. Do not touch version.c.SH or version.h, unless you are a head coder - if you need a credit in, contact us - -14. Protocol changes must be discussed before making patches for it. - -15. We do NOT rip people off. If we use other people's code, it MUST be - properly credited. - -16. We generally use tabsize 4 and 8. In any case, use tabs and NOT spaces. - Some code is old and horrible and has a mix of tabs and spaces used for - spacing, that's something we do not want to have ;) - -17. Be careful about overflows. Do not do any unchecked string copies. - Instead of strcpy, strcat and sprintf/ircsprintf, use the following - functions: strlcpy, strlcat, snprintf/ircnsprintf. - If you are copying/writing character-by-character or word-by-word in a - loop, be very sure about your size counting. Sometimes it's possible - to avoid such code alltogether by just calling strlcat each time. - -18. Speed. When optimizing or writing code, keep in mind that readability and - stability comes FIRST, and after that comes speed. So we'd rather prefer some - readable code (even if difficult) over some odd highly optimized routine which - nobody understands, is difficult to extend, and might have several bugs. - As mentioned earlier: use ircsnprintf, not snprintf (this is because - ircsnprintf is optimized for simple strings like the ones we use). - ircsnprintf calls snprintf when it finds a (non-simple) format specifier it - can't handle. Simple format specifiers do not have prefixes other than - h and l. - -19. Initialize your structs and use the proper memory calls. - In UnrealIRCd we use MyMalloc, MyMallocEx and MyFree (so not malloc/free). - MyMalloc usually maps to malloc, and MyMallocEx is a malloc plus filling - the memory area (eg: the struct) with zero's (a la calloc). - Use of MyMallocEx is suggested. In general you should not be using MyMalloc. - "But MyMalloc is faster!" you might say. This is true, but using MyMallocEx - has very little speed impact and enormous benefits: people tend to forget - to set certain fields in the struct to NULL, or much more common: when - someone later on (eg: 1 year later) adds a field to a struct, there could - be several places he/she needs to update to make sure x->something is NULL - after allocating a new struct. Bad idea. - Little speed impact, huge stability benefits, easy decision ;). - -20. Comment your code! This should speak for itself... - Put comments wherever you think they are needed, to aid any further coders - with reading your code.. and, in fact, it will aid yourself as well if you - would look back at your code 2 years later. - If there's some obscure pitfall, DO mention it! Don't just "hope" a next - author will see it like you did. - -21. Use enums whenever possible, rather than #define constants. Besides making - things more clean, it also aids debugging. +Rules about patches & modifications to UnrealIRCd + +1. When making a change, always add a small description in the commit log. + Don't forget to mention the bug# and credit the reporter (if any). + +2. If new files are made, they must contain proper copyright headers. + +3. Each bug or feature should have a bug# so people can have a discussion + about it. This has a few implications (read!!): + * People must report bugs/feature requests to bugs.unrealircd.org and + not on IRC, e-mail, etc. + * That means other people can see the bug# and comment on it. This means + discussion is easy to read back for each issue and not spread between + several IRC logs. + Furthermore, by using the bugtracker instead of directly committing, + people could point out that there might be a better way to do things + than you originally thought, or it might be that other devs don't like + it at all. + * If a head coder has 'acknowledged' or 'confirmed' the issue or stated + in a comment that it's OK to implement, then any dev may take the issue. + The dev should change the status to 'assigned' and work on it, then + commit and change it to 'resolved', set 'fixed in version' to the + correct release, and add a comment pasting the relevant commit log. + Of course other guidelines, in particular rule #7, still applies. + +4. If you don't have direct write access to the repository then you can + submit changes as as PR on github. It is very much preferred to also + have a bugs.unrealircd.org entry for it as well (see previous item). + +5. For the stable branch, in general, only commit changes that have an + associated bugid# and/or were discussed. + For branches currently in development (alpha/beta) there's more freedom + and if you think the change will be small and is fine without a + discussion then feel free to commit. + +6. Regarding reidenting, restructuring or other major code cleanups: please + discuss before doing so. The other devs might not agree with you on the + particular cleanup you have in mind which would result in another + clean-up-the-cleanup commit. + You may, however reindent and clean up individual sections when you are + working on fixing a particular bug# or implementing a new feature. In fact + you're encouraged to do so if the code is confusing without it. However, + obey the style of Unreal's code (mostly outlined in this document) + and do not introduce yet another (new) style. Also, be careful with doing + any cleanup: if you're unsure in any way about the use of something, + or something that looks redundant on first sight, then look more + carefully... it might indeed be useless and/or redundant, but it might + also be a subtle thing that can create great bugs when 'cleaned up'. + +7. During the Release Candidate stage (from RC1 until the final release) + only the head coder may commit directly, all others should ask and + present their patch before committing. Yes, even if you are changing + only 1 line of code or text. + +9. UnrealIRCd should compile on all supported operating systems and + platforms, using GCC 3 or higher on *NIX, and Visual Studio 2008 or + higher on Windows. This means you cannot blindly use all C99 extensions. + +10. Coders must test their code before committing. + +11. /* + * These kind of comments + */ + + NOT + + // These kind of comments + +12. if (something == 1) + { + moo; /* comment */ + /* This does what what what */ + cow(go(moo)); + } + + NOT + + if (something == 1) { + } + +13. Do not touch version.c.SH or version.h, unless you are a head coder. + If you need a credit in, contact us + +14. Protocol changes must be discussed before making patches for it. + +15. We do NOT rip people off. If we use other people's code, it MUST be + properly credited. + +16. We use tabsize 8 and we use tabs AND NOT SPACES. + Some code is old and horrible and has a mix of tabs and spaces used for + spacing, that's something we do not want to have ;) + +17. Be careful about overflows. Do not do any unchecked string copies. + Instead of strcpy, strcat and sprintf/ircsprintf, use the following + functions: strlcpy, strlcat, snprintf/ircnsprintf. + If you are copying/writing character-by-character or word-by-word in a + loop, eg using *p++ = x; then be very sure about your size counting. + Often it's better to avoid such code altogether, by simply using + strlcat for everything. + +18. Speed. When optimizing or writing code, keep in mind that readability and + stability comes FIRST, and after that comes speed. So we'd rather prefer some + readable code (even if difficult) over some odd highly optimized routine which + nobody understands, is difficult to extend, and might have several bugs. + As mentioned earlier: use ircsnprintf, not snprintf (this is because + ircsnprintf is optimized for simple strings like the ones we use). + ircsnprintf calls snprintf when it finds a (non-simple) format specifier it + can't handle. Simple format specifiers do not have prefixes other than + h and l. + +19. Initialize your structs and use the proper memory calls. + In UnrealIRCd we use MyMalloc, MyMallocEx and MyFree (so not malloc/free). + MyMalloc usually maps to malloc, and MyMallocEx is a malloc plus filling + the memory area (eg: the struct) with zero's (a la calloc). + Use of MyMallocEx is suggested. In general you should not be using MyMalloc. + "But MyMalloc is faster!" you might say. This is true, but using MyMallocEx + has very little speed impact and enormous benefits: people tend to forget + to set certain fields in the struct to NULL, or much more common: when + someone later on (eg: 1 year later) adds a field to a struct, there could + be several places he/she needs to update to make sure x->something is NULL + after allocating a new struct. Bad idea. + Little speed impact, huge stability benefits, easy decision ;). + +20. Comment your code! This should speak for itself... + Put comments wherever you think they are needed, to aid any further coders + with reading your code.. and, in fact, it will aid yourself as well if you + would look back at your code 2 years later. + If there's some obscure pitfall, DO mention it! Don't just "hope" a next + author will see it like you did. + +21. Use enums whenever possible, rather than #define constants. Besides making + things more clean, it also aids debugging.