diff --git a/docs/manuals/en/new_main_reference/source/developers/generaldevel.rst b/docs/manuals/en/new_main_reference/source/developers/generaldevel.rst index 211d1985986..49c16c6b65c 100644 --- a/docs/manuals/en/new_main_reference/source/developers/generaldevel.rst +++ b/docs/manuals/en/new_main_reference/source/developers/generaldevel.rst @@ -170,37 +170,53 @@ Contributions ------------- Contributions to the Bareos project come in many forms: ideas, -participation in helping people on the bareos-users email list, +participation in helping people on the `bareos-users`_ email list, packaging Bareos binaries for the community, helping improve the documentation, and submitting code. Patches ------- -Subject to the copyright assignment described below, your patches should -be sent in **git format-patch** format relative to the current contents -of the master branch of the Git repository. Please attach the output -file or files generated by the **git format-patch** to the email rather -than include them direct to avoid wrapping of the lines in the patch. -Please be sure to use the Bareos indenting standard (see below) for -source code. If you have checked out the source with Git, you can get a -diff using. +Patches should be sent as a pull-request to the master branch of the GitHub repository. +To do so, you will need an account on GitHub. +If you don't want to sign up to GitHub, you can also send us your patches via E-Mail in **git format-patch** format to the `bareos-devel`_ mailing list. + +Please make sure to use the Bareos `formatting standards`_. +Don’t forget any Copyrights and acknowledgments if it isn’t 100% your code. +Also, include the Bareos copyright notice that can be found in every source file. + +If you plan on doing significant development work over a period of time, after having your first patch reviewed and approved, you will be eligible for GitHub write access so that you can commit your changes directly into branches of the main GitHub repository. + +Commit message guideline +~~~~~~~~~~~~~~~~~~~~~~~~ +Start with a subject on a single line. +If your commit changes a specific component of bareos try to mention it at the start of the subject. + +Next comes an empty line. + +If your commit fixes an existing bug, add a line in the format ``Fixes #12345: The exact title of the bug you fix.``. +After this you can just write your detailed commit information. + +We strongly encourage you to keep the subject down to 50 characters and to wrap your text at the 50 character boundary. +However, this is by no means enforced. :: - git pull - git format-patch -M + lib: do a sample commit + + Fixes #12345: Really nasty library needs a sample commit. + + This patch fixes a bug in one of the libraries. + Before we applied this specific change, the + library was completely okay, but in desperate + need of a sample commit. -If you plan on doing significant development work over a period of time, -after having your first patch reviewed and approved, you will be -eligible for having developer Git write access so that you can commit -your changes directly to the Git repository. To do so, you will need a -userid on Source Forge. -Bugs Database -------------- -We have a bugs database which is at https://bugs.bareos.org. +Bug Database +------------ + +We have a bug database which is at https://bugs.bareos.org. The first thing is if you want to take over a bug, rather than just make a note, you should assign the bug to yourself. This helps other @@ -217,14 +233,14 @@ confirmed, or feedback when we first start working on the bug. Feedback is set when we expect that the user should give us more information. Normally, once you are reasonably sure that the bug is fixed, and a -patch is made and attached to the bug report, and/or in the SVN, you can +patch is made and attached to the bug report, and/or in the Git, you can close the bug. If you want the user to test the patch, then leave the bug open, otherwise close it and set **Resolution** to **Fixed**. We generally close bug reports rather quickly, even without confirmation, especially if we have run tests and can see that for us the problem is fixed. However, in doing so, it avoids misunderstandings if you leave a note while you are closing the bug that says something to the following -effect: We are closing this bug because … If for some reason, it does +effect: We are closing this bug because... If for some reason, it does not fix your problem, please feel free to reopen it, or to open a new bug report describing the problem“. @@ -234,13 +250,11 @@ if someone accidentally makes a bug note private, you should ask the reason and if at all possible (with his agreement) make the bug note public. -If the user has not properly filled in most of the important fields -(platorm, OS, Product Version, …) please do not hesitate to politely ask -him. Also, if the bug report is a request for a new feature, please -politely send the user to the Feature Request menu item on -www.bareos.org. The same applies to a support request (we answer only -bugs), you might give the user a tip, but please politely refer him to -the manual and the Getting Support page of www.bareos.org. +If the user has not properly filled in most of the important fields (platform, OS, Product Version, ...) please do not hesitate to politely ask him to do so. +The same applies to a support request (we answer only bugs), you might give the user a tip, but please politely refer him to the manual, the `bareos-users`_ mailing list and maybe the `commercial support`_. + +.. _bareos-users: https://groups.google.com/forum/#!forum/bareos-users +.. _commercial support: https://www.bareos.com/en/Support.html Developing Bareos ----------------- @@ -417,7 +431,7 @@ Debian-packages You can find the flags used for compiling for Debian in `debian/rules `__. -Rpm-packages +RPM-packages '''''''''''' You can find the flags used for compiling rpm-packages in @@ -442,12 +456,15 @@ usually be found quickly using a good multiple thread debugger such as **gdb**. For example, suppose you get a segmentation violation in **bareos-dir**. You might use the following to find the problem: - cd dird gdb ./bareos-dir run -f -s --c ./dird.conf where The **-f** -option is specified on the **run** command to inhibit **dird** from -going into the background. You may also want to add the **-s** option to -the run command to disable signals which can potentially interfere with -the debugging. +:: + + cd dird + gdb ./bareos-dir + run -f -s -c ./dird.conf + + +The **-f** option is specified on the **run** command to inhibit **dird** from going into the background. +You may also want to add the **-s** option to the run command to disable signals which can potentially interfere with the debugging. As an alternative to using the debugger, each **Bareos** daemon has a built in back trace feature when a serious error is encountered. It @@ -458,342 +475,85 @@ Crashes (Kaboom)”. Memory Leaks ~~~~~~~~~~~~ - -Because Bareos runs routinely and unattended on client and server -machines, it may run for a long time. As a consequence, from the very -beginning, Bareos uses SmartAlloc to ensure that there are no memory -leaks. To make detection of memory leaks effective, all Bareos code that -dynamically allocates memory MUST have a way to release it. In general -when the memory is no longer needed, it should be immediately released, -but in some cases, the memory will be held during the entire time that -Bareos is executing. In that case, there MUST be a routine that can be -called at termination time that releases the memory. In this way, we -will be able to detect memory leaks. Be sure to immediately correct any -and all memory leaks that are printed at the termination of the daemons. - -When Implementing Incomplete Code -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -Please identify all incomplete code with a comment that contains - -:: - - ***FIXME*** - -where there are three asterisks (*) before and after the word FIXME (in -capitals) and no intervening spaces. This is important as it allows new -programmers to easily recognize where things are partially implemented. - -Header Files -~~~~~~~~~~~~ - -Please carefully follow the scheme defined below as it permits in -general only two header file includes per C file, and thus vastly -simplifies programming. With a large complex project like Bareos, it -isn’t always easy to ensure that the right headers are invoked in the -right order (there are a few kludges to make this happen – i.e. in a few -include files because of the chicken and egg problem, certain references -to typedefs had to be replaced with **void** ). - -Every file should include **bareos.h**. It pulls in just about -everything, with very few exceptions. If you have system dependent -ifdefing, please do it in **baconfig.h**. The version number and date -are kept in **version.h**. - -Each of the subdirectories (console, cats, dird, filed, findlib, lib, -stored, …) contains a single directory dependent include file generally -the name of the directory, which should be included just after the -include of **bareos.h**. This file (for example, for the dird directory, -it is **dird.h**) contains either definitions of things generally needed -in this directory, or it includes the appropriate header files. It -always includes **protos.h**. See below. - -Each subdirectory contains a header file named **protos.h**, which -contains the prototypes for subroutines exported by files in that -directory. **protos.h** is always included by the main directory -dependent include file. - -Programming Standards -~~~~~~~~~~~~~~~~~~~~~ - -For the most part, all code should be written in C unless there is a -burning reason to use C++, and then only the simplest C++ constructs -will be used. Note, Bareos is slowly evolving to use more and more C++. - -Code should have some documentation – not a lot, but enough so that I -can understand it. Look at the current code, and you will see that I -document more than most, but am definitely not a fanatic. - -We prefer simple linear code where possible. Gotos are strongly -discouraged except for handling an error to either bail out or to retry -some code, and such use of gotos can vastly simplify the program. - -Remember this is a C program that is migrating to a **tiny** subset of -C++, so be conservative in your use of C++ features. - -Do Not Use -~~~~~~~~~~ - -- STL – it is totally incomprehensible. - -Avoid if Possible -~~~~~~~~~~~~~~~~~ - -- Using **void \*** because this generally means that one must using - casting, and in C++ casting is rather ugly. It is OK to use void \* - to pass structure address where the structure is not known to the - routines accepting the packet (typically callback routines). However, - declaring “void \*buf” is a bad idea. Please use the correct types - whenever possible. - -- Using undefined storage specifications such as (short, int, long, - long long, size_t …). The problem with all these is that the number - of bytes they allocate depends on the compiler and the system. - Instead use Bareos’s types (int8_t, uint8_t, int32_t, uint32_t, - int64_t, and uint64_t). This guarantees that the variables are given - exactly the size you want. Please try at all possible to avoid using - size_t ssize_t and the such. They are very system dependent. However, - some system routines may need them, so their use is often - unavoidable. - -- Returning a malloc’ed buffer from a subroutine – someone will forget - to release it. - -- Heap allocation (malloc) unless needed – it is expensive. Use - POOL_MEM instead. - -- Templates – they can create portability problems. - -- Fancy or tricky C or C++ code, unless you give a good explanation of - why you used it. - -- Too much inheritance – it can complicate the code, and make reading - it difficult (unless you are in love with colons) - -Do Use Whenever Possible -~~~~~~~~~~~~~~~~~~~~~~~~ - -- Locking and unlocking within a single subroutine. - -- A single point of exit from all subroutines. A goto is perfectly OK - to use to get out early, but only to a label named bail_out, and - possibly an ok_out. See current code examples. - -- malloc and free within a single subroutine. - -- Comments and global explanations on what your code or algorithm does. - -- When committing a fix for a bug, make the comment of the following - form: - - :: - - Fixes #1234: Description of the bug. - - Reason for bug fix - or other message. - - It is important to write the **bug #1234** like that because our - program that automatically pulls messages from the git repository to - make the ChangeLog looks for that pattern. Obviously the **1234** - should be replaced with the number of the bug you actually fixed. - - Providing the commit comment line has one of the following keywords - (or phrases), it will be ignored: - - :: - - tweak - typo - cleanup - bweb: - regress: - again - .gitignore - fix compilation - technotes - update version - update technotes - update projects - update releasenotes - update version - update home - update release - update todo - update notes - update changelog - -- Use the following keywords at the beginning of a git commit message - -Indenting Standards -~~~~~~~~~~~~~~~~~~~ - -We find it very hard to read code indented 8 columns at a time. Even 4 -at a time uses a lot of space, so we have adopted indenting 3 spaces at -every level. Note, indention is the visual appearance of the source on -the page, while tabbing is replacing a series of up to 8 spaces from a -tab character. - -The closest set of parameters for the Linux **indent** program that will -produce reasonably indented code are: - -:: - - indent -nbad -bap -bbo -nbc -br -brs -c36 -cd36 -ncdb -ce -ci3 -cli0 -cp36 -d0 -di1 -ndj -nfc1 -nfca -hnl -i3 -ip0 -l85 -lp -npcs -nprs -npsl -saf -sai -saw -nsob -nss -nbc -ncs -nbfda --no-tabs -T POOL_MEM -T json_t - -You can put the above in your .indent.pro file, and then just invoke -indent on your file. However, be warned. This does not produce perfect -indenting, and it will mess up C++ class statements pretty badly. - -Also typedefs/classes must be specified by the ``-T`` flag. - -Braces are required in all if statements (missing in some very old -code). To avoid generating too many lines, the first brace appears on -the first line (e.g. of an if), and the closing brace is on a line by -itself. E.g. - -:: - - if (abc) { - some_code; - } - -Just follow the convention in the code. For example we I prefer -non-indented cases. - -:: - - switch (code) { - case 'A': - do something - break; - case 'B': - again(); - break; - default: - break; - } - -Avoid using // style comments except for temporary code or turning off -debug code. Standard C comments are preferred (this also keeps the code -closer to C). - -Attempt to keep all lines less than 85 characters long so that the whole -line of code is readable at one time. This is not a rigid requirement. - -Always put a brief description at the top of any new file created -describing what it does and including your name and the date it was -first written. Please don’t forget any Copyrights and acknowledgments if -it isn’t 100% your code. Also, include the Bareos copyright notice that -is in **src/c**. - -In general you should have two includes at the top of the an include for -the particular directory the code is in, for includes are needed, but -this should be rare. - -In general (except for self-contained packages), prototypes should all -be put in **protos.h** in each directory. - -Always put space around assignment and comparison operators. - -:: - - a = 1; - if (b >= 2) { - cleanup(); - } - -but your can compress things in a **for** statement: - -:: - - for (i=0; i < del.num_ids; i++) { - ... - -Don’t overuse the inline if (?:). A full **if** is preferred, except in -a print statement, e.g.: - -:: - - if (ua->verbose \&& del.num_del != 0) { - bsendmsg(ua, _("Pruned %d %s on Volume %s from catalog.\n"), del.num_del, - del.num_del == 1 ? "Job" : "Jobs", mr->VolumeName); - } - -Leave a certain amount of debug code (Dmsg) in code you submit, so that -future problems can be identified. This is particularly true for -complicated code likely to break. However, try to keep the debug code to -a minimum to avoid bloating the program and above all to keep the code -readable. - -Please keep the same style in all new code you develop. If you include -code previously written, you have the option of leaving it with the old -indenting or re-indenting it. If the old code is indented with 8 spaces, -then please re-indent it to Bareos standards. - -If you are using **vim**, simply set your tabstop to 8 and your -shiftwidth to 3. - -Tabbing -~~~~~~~ - -Tabbing (inserting the tab character in place of spaces) is as normal on -all Unix systems – a tab is converted space up to the next column -multiple of 8. My editor converts strings of spaces to tabs -automatically – this results in significant compression of the files. -Thus, you can remove tabs by replacing them with spaces if you wish. -Please don’t confuse tabbing (use of tab characters) with indenting -(visual alignment of the code). - -Don’ts +Most of Bareos still uses SmartAlloc. +This tracks memory allocation and allows you to detect memory leaks. +However, newer code should not use SmartAlloc, but use standard C++11 resource management (RAII). +If you need to detect memory leaks, you can just use ``valgrind`` to do so. + +Guiding Principles +~~~~~~~~~~~~~~~~~~ +All new code should be written in modern C++11 following the `Google C++ Style Guide`_ and the `C++ Core Guidelines`_. + +We consider simple better than complex, but complex code is still better than complicated code. + +Currently there is still a lot of old C++ and C code in the code base and a lot of existing code violates our `do's`_ and `don'ts`_. +Our long-term goal is to modernize the code-base to make it easier to understand, easier to maintain and better approachable for new developers. + +Formatting Standards +~~~~~~~~~~~~~~~~~~~~ + +We find it very hard to adapt to different styles of code formatting, so we agreed on a set of rules. +Instead of describing these in a lenghty set of rules, we provide a configuration file for **clang-format** in our repository that we use to format the code. +All code should be reformatted using **clang-format** before committing. + +For some parts of code it works best to hand-optimize the formatting. +We sometimes do this for larger tables and deeply nested brace initialization. +If you need to hand-optimize make sure you add **clang-format off** and **clang-format on** comments so applying **clang-format** on your source will not undo your manual optimization. +Please apply common sense and use this exception sparingly. + +Prefer ``/* */`` for code comments. +You can use ``//`` for single-line comments. + +Do's +~~~~ +- write modern C++11 +- prefer simple code +- write unit tests for your code +- use RAII_ whenever possible +- honor `Rule of three`_/`Rule of five`/`Rule of zero` +- use ``std::string`` instead of ``char*`` for strings where possible +- use `fixed width integer types`_ if the size of your integer matters +- when in doubt always prefer the standard library above a custom implementation +- follow the `Google C++ Style Guide`_ +- follow the `C++ Core Guidelines`_ +- get in touch with us on the `bareos-devel`_ mailing list + +.. _RAII: https://en.cppreference.com/w/cpp/language/raii +.. _Rule of three: https://en.cppreference.com/w/cpp/language/rule_of_three +.. _fixed width integer types: https://en.cppreference.com/w/cpp/types/integer +.. _Google C++ Style Guide: https://google.github.io/styleguide/cppguide.html +.. _C++ Core Guidelines: http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines +.. _bareos-devel: https://groups.google.com/forum/#!forum/bareos-devel + +Don'ts ~~~~~~ +avoid ``new`` + We provide a backport of C++14's ``make_unique()`` in ``make_unique.h``. + Whereever possible this should be used instead of ``new``. -Please don’t use: +avoid ``delete`` + Cleanup should be handled automatically with RAII. -:: +don't transfer ownership of heap memory without move semantics + No returning of raw pointers where the caller is supposed to free() - strcpy() - strcat() - strncpy() - strncat(); - sprintf() - snprintf() +don't use C++14 or later + Currently we support platforms where the newest available compiler supports only C++11. -They are system dependent and un-safe. These should be replaced by the -Bareos safe equivalents: +don't use C string functions + If you can, use ``std::string`` and don't rely on C's string functions. + If you can't use the bareos replacement for the string function. + This is usually just prefixed with a "b". + These replacements will play nicely with the (now deprecated) smart allocator. -:: +avoid the ``edit_*()`` functions from ``edit.cc`` + Just use the appropriate format string. + This will also avoid the temporary buffer that is required otherwise. - char *bstrncpy(char *dest, char *source, int dest_size); - char *bstrncat(char *dest, char *source, int dest_size); - int bsnprintf(char *buf, int32_t buf_len, const char *fmt, ...); - int bvsnprintf(char *str, int32_t size, const char *format, va_list ap); - -See src/lib/bsys.c for more details on these routines. - -Don’t use the **%lld** or the **%q** printf format editing types to edit -64 bit integers – they are not portable. Instead, use **%s** with -**edit_uint64()**. For example: - -:: +don't subclass ``SmartAlloc`` + It forces the use of ancient memory management methods and severely limits the capabilities of your class - char buf[100]; - uint64_t num = something; - char ed1[50]; - bsnprintf(buf, sizeof(buf), "Num=%s\n", edit_uint64(num, ed1)); - -Note: **%lld** is now permitted in Bareos code – we have our own printf -routines which handle it correctly. The edit_uint64() subroutine can -still be used if you wish, but over time, most of that old style will be -removed. - -The edit buffer **ed1** must be at least 27 bytes long to avoid -overflow. See src/lib/edit.c for more details. If you look at the code, -don’t start screaming that I use **lld**. I actually use subtle trick -taught to me by John Walker. The **lld** that appears in the editing -routine is actually **#define** to a what is needed on your OS (usually -“lld” or “q”) and is defined in autoconf/configure.in for each OS. C -string concatenation causes the appropriate string to be concatenated to -the “%”. - -Also please don’t use the STL or Templates or any complicated C++ code. +avoid smart allocation + The whole smart allocation library with ``get_pool_memory()``, ``sm_free()`` and friends does not mix with RAII, so we will try to remove them step by step in the future. + Avoid in new code if possible. +