566 lines
22 KiB
Text
566 lines
22 KiB
Text
Information for developers
|
||
==========================
|
||
|
||
This document is intended to explain some of the more useful things
|
||
within the tree, and provide a standard for working on the code.
|
||
|
||
General stuff - common subdirectory
|
||
-----------------------------------
|
||
|
||
String handling
|
||
~~~~~~~~~~~~~~~
|
||
|
||
Use snprintf(). It's even provided with a compatibility module if the
|
||
target system doesn't have it natively.
|
||
|
||
If you use snprintf() to load some value into a buffer, make sure you
|
||
provide the format string. Don't use user-provided format strings,
|
||
since that's an easy way to open yourself up to an exploit.
|
||
|
||
Don't use strcat(). We have a neat wrapper for snprintf() called snprintfcat()
|
||
that allows you to append to char * with a format string and all the usual
|
||
string length checking of snprintf().
|
||
|
||
Error reporting
|
||
~~~~~~~~~~~~~~~
|
||
|
||
Don't call syslog() directly. Use upslog_with_errno() and upslogx().
|
||
They may write to the syslog, stderr, or both as appropriate. This
|
||
means you don't have to worry about whether you're running in the
|
||
background or not.
|
||
|
||
upslog_with_errno prints your message plus the string expansion of
|
||
errno. upslogx just prints the message.
|
||
|
||
fatal_with_errno and fatalx work the same way, but they
|
||
exit(EXIT_FAILURE) afterwards. Don't call exit() directly.
|
||
|
||
Debugging information
|
||
~~~~~~~~~~~~~~~~~~~~~
|
||
|
||
upsdebug_with_errno(), upsdebugx(), upsdebug_hex() and upsdebug_ascii()
|
||
use the global `nut_debug_level` so you don't have to mess around with
|
||
printf()s yourself. Use them.
|
||
|
||
Memory allocation
|
||
~~~~~~~~~~~~~~~~~
|
||
|
||
xmalloc, xcalloc, xrealloc and xstrdup all check the results of the base
|
||
calls before continuing, so you don't have to. Don't use the raw calls
|
||
directly.
|
||
|
||
Config file parsing
|
||
~~~~~~~~~~~~~~~~~~~
|
||
|
||
The configuration parser, called parseconf, is now up to its fourth
|
||
major version. It has multiple entry points, and can handle many
|
||
different jobs. It's usually used for parsing files, but it can also
|
||
take input a line at a time or even a character at a time.
|
||
|
||
You must initialize a context buffer with pconf_init before using any
|
||
other parseconf function. pconf_encode is the only exception, since it
|
||
operates on a buffer you supply and is an auxiliary function.
|
||
|
||
Escaping special characters and quoting multiple-word elements is all
|
||
handled by the state machine. Using the same code for all config files
|
||
avoids code duplication.
|
||
|
||
NOTE: this does not apply to drivers. Driver authors should use the
|
||
upsdrv_makevartable() scheme to pick up values from ups.conf. Drivers
|
||
should not have their own config files.
|
||
|
||
Drivers may have their own data files, such as lists of hardware,
|
||
mapping tables, or similar. The difference between a data file and a
|
||
config file is that users should never be expected to edit a data file
|
||
under normal circumstances. This technique might be used to add more
|
||
hardware support to a driver without recompiling.
|
||
|
||
<time.h> vs. <sys/time.h>
|
||
~~~~~~~~~~~~~~~~~~~~~~~~~
|
||
|
||
This is already handled by autoconf, so just include "timehead.h" and you
|
||
will get the right headers on every system.
|
||
|
||
Device drivers - main.c
|
||
-----------------------
|
||
|
||
The device drivers use main.c as their core. The only exceptions are the
|
||
HAL-based drivers, which use the same dstate function calls while integrating
|
||
with the DBUS event loop.
|
||
|
||
To write a new driver, you create a file with a series of support
|
||
functions that will be called by main. These all have names that start
|
||
with `upsdrv_`, and they will be called at different times by main
|
||
depending on what needs to happen.
|
||
|
||
See the <<new-drivers,driver documentation>> for information on writing
|
||
drivers, and also refer to the skeletal driver in skel.c.
|
||
|
||
Portability
|
||
-----------
|
||
|
||
Avoid things that will break on other systems. All the world is not an
|
||
x86 Linux box.
|
||
|
||
There are still older systems out there that don't do C++ style comments.
|
||
|
||
--------------------------------------
|
||
/* Comments look like this. */
|
||
// Not like this.
|
||
--------------------------------------
|
||
|
||
Newer versions of gcc allow you to declare a variable inside a function
|
||
somewhat like the way C++ operates, like this:
|
||
|
||
--------------------------------------------------------------------------------
|
||
function do_stuff(void)
|
||
{
|
||
check_something();
|
||
|
||
int a;
|
||
|
||
a = do_something_else();
|
||
}
|
||
--------------------------------------------------------------------------------
|
||
|
||
While this will compile and run on these newer versions, it will fail
|
||
miserably for anyone on an older system. That means you must not use
|
||
it. gcc only warns about this with -pedantic.
|
||
|
||
Coding style
|
||
------------
|
||
|
||
This is how we do things:
|
||
|
||
--------------------------------------------------------------------------------
|
||
int open_subspace(char *ship, int privacy)
|
||
{
|
||
if (!privacy)
|
||
return insecure_channel(ship);
|
||
|
||
if (!init_privacy(ship))
|
||
fatal_with_errno("Can't open secure channel");
|
||
|
||
return secure_channel(ship);
|
||
}
|
||
--------------------------------------------------------------------------------
|
||
|
||
The basic idea is that we try to group things into functions, and then
|
||
find ways to drop out of them when we can't go any further. There's
|
||
another way to program this involving a big else chunk and a bunch of
|
||
braces, and it can be hard to follow. You can read this from top to
|
||
bottom and have a pretty good idea of what's going on without having to
|
||
track too much { } nesting and indenting.
|
||
|
||
We don't really care for pretentiousVariableNamingSchemes, but you can
|
||
probably get away with it in your own driver that we will never have to
|
||
touch. If your function or variable names start pushing important code
|
||
off the right margin of the screen, expect them to meet the byte
|
||
chainsaw sooner or later.
|
||
|
||
All types defined with typedef should end in "_t", because this is
|
||
easier to read, and it enables tools (such as indent and emacs) to
|
||
display the source code correctly.
|
||
|
||
Indenting with tabs vs. spaces
|
||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||
|
||
Another thing to notice is that the indenting happens with tabs instead
|
||
of spaces. This lets everyone have their personal tab-width setting
|
||
without inflicting much pain on other developers. If you use a space,
|
||
then you've fixed the spacing in stone and have really annoyed half of
|
||
the people out there.
|
||
|
||
Note that tabs apply only to *indenting*. Alignment of text after any
|
||
non-tab character has appeared on the line must be done by spaces in
|
||
order for it to remain at the same alignment when someone views tabs at
|
||
a different widths.
|
||
|
||
If you write something that uses spaces, you may get away with it in a
|
||
driver that's relatively secluded. However, if we have to work on that
|
||
code, expect it to get reformatted according to the above.
|
||
|
||
Patches to existing code that don't conform to the coding style being
|
||
used in that file will probably be dropped. If it's something we really
|
||
need, it will be grudgingly reformatted before being included.
|
||
|
||
When in doubt, have a look at Linus's take on this topic in the Linux
|
||
kernel - Documentation/CodingStyle. He's done a far better job of
|
||
explaining this.
|
||
|
||
Line breaks
|
||
~~~~~~~~~~~
|
||
|
||
It is better to have lines that are longer than 80 characters than to
|
||
wrap lines in random places. This makes it easier to work with tools
|
||
such as "grep", and it also lets each developer choose their own
|
||
window size and tab setting without being stuck to one particular
|
||
choice.
|
||
|
||
Of course, this does not mean that lines should be made unnecessarily
|
||
long when there is a better alternative (see the note on
|
||
pretentiousVariableNamingSchemes above). Certainly there should not
|
||
be more than one statement per line. Please do not use
|
||
|
||
--------------------------------------------------------------------------------
|
||
if (condition) break;
|
||
--------------------------------------------------------------------------------
|
||
|
||
but use the following:
|
||
|
||
--------------------------------------------------------------------------------
|
||
if (condition) {
|
||
break;
|
||
}
|
||
--------------------------------------------------------------------------------
|
||
|
||
Miscellaneous coding style tools
|
||
--------------------------------
|
||
|
||
You can go a long way towards converting your source code to the NUT
|
||
coding style by piping it through the following command:
|
||
|
||
indent -kr -i8 -T FILE -l1000 -nhnl
|
||
|
||
This next command does a reasonable job of converting most C++ style
|
||
comments (but not URLs and DOCTYPE strings):
|
||
|
||
sed 's#\(^\|[ \t]\)//[ \t]*\(.*\)[ \t]*#/* \2 */#'
|
||
|
||
Emacs users can adjust how tabs are displayed. For example, it is
|
||
possible to set a tab stop to be 3 spaces, rather than the usual 8.
|
||
(Note that in the saved file, one indentation level will still
|
||
correspond to one tab stop; the difference is only how the file is
|
||
rendered on screen). It is even possible to set this on a
|
||
per-directory basis, by putting something like this into your .emacs
|
||
file:
|
||
|
||
--------------------------------------------------------------------------------
|
||
;; NUT style
|
||
|
||
(defun nut-c-mode ()
|
||
"C mode with adjusted defaults for use with the NUT sources."
|
||
(interactive)
|
||
(c-mode)
|
||
(c-set-style "K&R")
|
||
(setq c-basic-offset 3) ;; 3 spaces C-indentation
|
||
(setq tab-width 3)) ;; 3 spaces per tab
|
||
|
||
;; apply NUT style to all C source files in all subdirectories of nut/
|
||
|
||
(setq auto-mode-alist (cons '(".*/nut/.*\\.[ch]$". nut-c-mode)
|
||
auto-mode-alist))
|
||
--------------------------------------------------------------------------------
|
||
|
||
Finishing touches
|
||
~~~~~~~~~~~~~~~~~
|
||
|
||
We like code that uses const and static liberally. If you don't need to
|
||
expose a function or global variable to the outside world, static is
|
||
your friend. If nobody should edit the contents of some buffer that's
|
||
behind a pointer, const keeps them honest.
|
||
|
||
We always compile with -Wall, so things like const and static help you
|
||
find implementation flaws. Functions that attempt to modify a constant
|
||
or access something outside their scope will throw a warning or even
|
||
fail to compile in some cases. This is what we want.
|
||
|
||
|
||
Spaghetti
|
||
~~~~~~~~~
|
||
|
||
If you use a goto, expect us to drop it when our head stops spinning.
|
||
It gives us flashbacks to the very old code we wrote.
|
||
We've tried to clean up our act, and you should make the effort
|
||
as well.
|
||
|
||
We're not making a blanket statement about gotos, since everything
|
||
probably has at least one good use. There are a few cases where a goto
|
||
is more efficient than any other approach, but you probably won't
|
||
encounter them very often in this software.
|
||
|
||
|
||
Legacy code
|
||
~~~~~~~~~~~
|
||
|
||
There are parts of the source tree that do not yet conform to these
|
||
specs. Part of this is due to the fact that the coding style has been
|
||
evolving slightly over the course of the project. Some of the code you
|
||
see in these directories is 5 years old, and things have gotten cleaner
|
||
since then. Don't worry - it'll get cleaned up the next time something
|
||
in the vicinity gets a visit.
|
||
|
||
Memory leak checking
|
||
~~~~~~~~~~~~~~~~~~~~
|
||
|
||
We can't say enough good things about valgrind. If you do anything with
|
||
dynamic memory in your code, you need to use this. Just compile with -g
|
||
and start the program inside valgrind. Run it through the suspected
|
||
area and then exit cleanly. valgrind will tell you if you've done
|
||
anything dodgy like freeing regions twice, reading uninitialized memory,
|
||
or if you've leaked memory anywhere.
|
||
|
||
For more information, refer to the link:http://valgrind.kde.org[Valgrind]
|
||
project.
|
||
|
||
Conclusion
|
||
~~~~~~~~~~
|
||
|
||
The summary: please be kind to our eyes. There's a lot of stuff in here,
|
||
and many people have put a lot of time and energy to improve it.
|
||
|
||
Submitting patches
|
||
------------------
|
||
|
||
Small patches that arrive in unified format (diff -u) as plain text attachments
|
||
with no HTML and a brief summary at the top are the easiest to handle.
|
||
|
||
If a patch is sent to the nut-upsdev mailing list, it stands a better chance of
|
||
being seen immediately. However, it is likely to be dropped if any issues
|
||
cannot be resolved quickly. If your code might not work for others, or if it is
|
||
a large change, your best bet is to submit a pull request or create an
|
||
link:https://github.com/networkupstools/nut/issues[issue on GitHub].
|
||
|
||
The issue tracker allows us to track the patches over a longer period of time,
|
||
and it is less likely that a patch will fall through the cracks. Posting a
|
||
reminder to the developers (via the nut-upsdev list) about a patch on GitHub is
|
||
fair game.
|
||
|
||
Patch cohesion
|
||
--------------
|
||
|
||
Patches should have some kind of unifying element. One patch set is one
|
||
message, and it should all touch similar things. If you have to edit 6
|
||
files to add support for neutrino detection in UPS hardware, that's
|
||
fine.
|
||
|
||
However, sending one huge patch that does massive separate changes all over
|
||
the tree is not recommended. That kind of patch has to be split up and
|
||
evaluated separately, assuming the core developers care enough to do that
|
||
instead of just dropping it.
|
||
|
||
If you have to make big changes in lots of places, send multiple
|
||
patches - one per item.
|
||
|
||
The finishing touches: manual pages and device entry in HCL
|
||
-----------------------------------------------------------
|
||
|
||
If you change something that involves an argument to a program or
|
||
configuration file parsing, the man page is probably now out of date.
|
||
If you don't update it, we have to, and we have enough to do as it is.
|
||
|
||
If you write a new driver, send in the man page when you send us the
|
||
source code for your driver. Otherwise, we will be forced to write a
|
||
skeletal man page that will probably miss many of the finer points of
|
||
the driver and hardware.
|
||
|
||
The same remark goes for device entries: if you add support for new models,
|
||
remember to also complete the hardware compatibility list, present
|
||
in data/driver.list.in. This will be used to generate both textual, static
|
||
HTML and dynamic searchable HTML for the website.
|
||
|
||
Source code management
|
||
----------------------
|
||
|
||
We currently use a Git repository hosted at GitHub (with a mirror at Alioth) to track
|
||
changes to the NUT source code. This allows you to clone the repository (or
|
||
fork, in GitHub parlance), make changes, and post them online for review prior
|
||
to integration.
|
||
|
||
To obtain permission to commit directly to the master NUT repository, you must
|
||
be prepared to spend a fair amount of time contributing to the NUT codebase.
|
||
Most developers will be well served by committing to their own Git repository,
|
||
and having the NUT team merge their changes.
|
||
|
||
Git offers a little more flexibility than the +svn update+ command. You may
|
||
fetch other developers' changes from SVN into your repository, but hold off on
|
||
actually combining them with your branch until you have compared the two
|
||
branches (for instance, with `gitk --all`). Git also allows you to accumulate
|
||
more than one commit worth of changes before pushing to another repository.
|
||
|
||
For a quick change to a file in the Git working copy, you can use `git diff` to
|
||
generate a patch to send to the nut-upsdev mailing list. If you have more
|
||
extensive changes, you can use `git format-patch` on a complete commit or
|
||
branch, and send the resulting series of patches to the list.
|
||
|
||
The link:https://git.wiki.kernel.org/index.php/GitSvnCrashCourse[GitSvnCrashCourse]
|
||
wiki page has some useful information for long-time users of Subversion.
|
||
|
||
Git access
|
||
~~~~~~~~~~
|
||
|
||
Anonymous Git checkouts are possible:
|
||
|
||
git clone git://github.com/networkupstools/nut.git
|
||
|
||
or
|
||
|
||
git clone https://github.com/networkupstools/nut.git
|
||
|
||
if it is necessary to get around a pesky firewall that blocks the native Git
|
||
protocol.
|
||
|
||
For a quicker checkout (when you don't need the entire repository history),
|
||
you can limit the depth of the clone:
|
||
|
||
git clone --depth 1 git://github.com/networkupstools/nut.git
|
||
|
||
In case the GitHub repository is temporarily unavailable for any reason, we
|
||
also plan to push to Alioth's
|
||
link:https://alioth.debian.org/scm/?group_id=30602[Git server] as well. You can
|
||
add a remote reference to your local repository:
|
||
|
||
cd path/to/nut
|
||
git remote add -f alioth git://anonscm.debian.org/nut/nut.git
|
||
|
||
Mercurial (hg) access
|
||
~~~~~~~~~~~~~~~~~~~~~
|
||
|
||
There are those who prefer the simplicity and self-consistency of the Mercurial
|
||
SCM client over the hodgepodge of unique commands which make up Git. Rather
|
||
than debate the merits of each system, we will gently guide you towards the
|
||
link:http://hg-git.github.com/[hg-git project] which would theoretically be a
|
||
transparent bridge between the central Git repository, and your local Mercurial
|
||
working copy.
|
||
|
||
Other tools for hg/git interoperability are sure to exist. We would welcome any
|
||
feedback about this process on the nut-upsdev mailing list.
|
||
|
||
Subversion (SVN) access
|
||
~~~~~~~~~~~~~~~~~~~~~~~
|
||
|
||
If you prefer to check out the NUT source code using an SVN client, GitHub
|
||
has a link:https://github.com/blog/966-improved-subversion-client-support[SVN
|
||
interface to Git repositories] hosted on their servers. You can fork a copy of
|
||
the NUT repository and commit to your fork with SVN.
|
||
|
||
Be aware that the examples in the GitHub blog post might result in a checkout
|
||
that includes all of the current branches, as well as the trunk. You are most
|
||
likely interested in a command line similar to the following:
|
||
|
||
svn co https://github.com/networkupstools/nut/trunk nut-trunk-svn
|
||
|
||
Ignoring generated files
|
||
------------------------
|
||
|
||
The NUT repository generally only holds files which are not generated from
|
||
other files. This prevents spurious differences from being recorded in the
|
||
repository history.
|
||
|
||
If you add a driver, it is recommended that you add the driver executable name
|
||
to the .gitignore file in that directory. Similarly, files generated from *.in
|
||
and *.am sources should be ignored as well. We try to include a number of
|
||
generated files in the tarball releases with `make dist` hooks in order to
|
||
minimize the number of dependencies for end users, but the assumption is that
|
||
a developer can install the packages needed to regenerate those files.
|
||
|
||
Commit message formatting
|
||
-------------------------
|
||
|
||
From the `git commit` man page:
|
||
|
||
[quote]
|
||
Though not required, it’s a good idea to begin the commit message with a single
|
||
short (less than 50 character) line summarizing the change, followed by a blank
|
||
line and then a more thorough description. The text up to the first blank line
|
||
in a commit message is treated as the commit title, and that title is used
|
||
throughout git.
|
||
|
||
If your commit is just a change to one component, such as the HCL, upsd or a
|
||
specific driver, prefix your commit message in a way that matches similar
|
||
commits. This helps when searching the repository or tracking down a
|
||
regression.
|
||
|
||
Referring to previous commits can be tricky. If you are referring to the
|
||
immediate parent of a given commit, it suffices to say "the previous commit".
|
||
(Are you correcting a typo in the previous commit? If you haven't pushed yet,
|
||
consider using the `git commit --amend` command instead of creating a new
|
||
commit.) For other commits, even though tools like gitk and GitHub's
|
||
repository viewers recognize Git hashes and create links automatically, it is
|
||
best to add some context such as the commit title or a date.
|
||
|
||
You may notice that some older commits have `[[SVN:####]]` tags and Fossil-ID
|
||
footers. These were lifted from the old SVN commit messages using reposurgeon,
|
||
and should not be used as a guide for future commits.
|
||
|
||
Repository etiquette and quality assurance
|
||
------------------------------------------
|
||
|
||
Please keep the Git master branch in working condition at all times. The
|
||
master branch may be used to generate daily tarballs, and should not contain
|
||
broken code. If you need to commit incremental changes that leave the system
|
||
in a broken state, please do so in a separate branch and merge the changes
|
||
back into master once they are complete.
|
||
|
||
You are encouraged to use `git rebase -i` on your private Git branches to
|
||
separate your changes into <<_patch_cohesion,logical changes>>.
|
||
|
||
From there, you can generate patches for the issue tracker, or the nut-upsdev list.
|
||
|
||
Note that once you rebase a branch, anyone else who has a copy of this branch
|
||
will need to rebase on top of your rebased branch. Obviously, this hinders
|
||
collaboration. In this case, we recommend that you rebase only in your private
|
||
repository, and push when things are ready for discussion. Merging instead of
|
||
rebasing will help with collaboration, but please do not turn the repository
|
||
history into a pile of spaghetti by merging unnecessarily. Be sure that your
|
||
commit messages are descriptive when merging.
|
||
|
||
Before pushing your commits upstream, please remember to run +make
|
||
distcheck-light+. This checks that the Makefiles are not broken, that all the
|
||
relevant files are distributed, and that there are no compilation or
|
||
installation errors.
|
||
|
||
Running +make distcheck-light+ is especially important if you have added or
|
||
removed files, or updated configure.in or some Makefile.am. Remember: simply
|
||
adding a file to Git does not mean it will be distributed. To distribute a
|
||
file, you must update the corresponding Makefile.am.
|
||
|
||
There is also +make distcheck+, which runs an even stricter set of
|
||
tests than +make distcheck-light+, but will not work unless you have all the
|
||
optional libraries and features installed.
|
||
|
||
You may create as many branches as you like in your local Git repository. When
|
||
using Git, our preferred way to combine small changes with the upstream
|
||
upstream repository is to use `git rebase` on your local branch. This is
|
||
equivalent to treating your branch as a series of patches, and re-applying your
|
||
patches on top of the upstream changes.
|
||
|
||
If you haven't created a commit out of your local changes yet, and you want to
|
||
fetch the latest code, you can also use +git stash+ before pulling, then +git
|
||
stash pop+ to apply your saved changes.
|
||
|
||
Here is an example workflow:
|
||
|
||
--------------------------------------------------------------------------------
|
||
git clone -o central git://github.com/networkupstools/nut.git
|
||
|
||
cd nut
|
||
git remote add -f username git://github.com/username/nut.git
|
||
|
||
git checkout master
|
||
git branch my-new-feature
|
||
git checkout my-new-feature
|
||
|
||
# Hack away
|
||
|
||
git add changed-file.c
|
||
git commit
|
||
|
||
# Fix a typo in a file or commit message:
|
||
|
||
git commit -a --amend
|
||
|
||
# Someone committed something to the central repository. Fetch it.
|
||
|
||
git fetch central
|
||
git rebase central/master
|
||
|
||
# Publish your branch to your GitHub repository:
|
||
|
||
git push username my-new-feature
|
||
--------------------------------------------------------------------------------
|
||
|
||
If you are new to Git, but are familiar with SVN, the
|
||
link:http://git-scm.com/course/svn.html[following link] may be of use.
|
||
|
||
|