We have multiple ways to define properties (like, GVariant based
nm_setting_option_*() or GObject based properties). For the latter,
they nowadays should all be implemented via _nm_setting_property_define_direct_*()
API.
Make checkpatch.pl identify subtree merges in "git am"-formatted
patches and reconstruct the full path names based in the subtree root.
This fixes some spurious warnings for parts of the tree that use
different coding style from what we usually do.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/989
The only reason is consistency. The majority of times we
do use g_snprintf(). As there are no strong reasons to
prefer one over the other, prefer the one that use use
most of the time.
g_idle_add() is discouraged, and the checkpatch.pl script warns
about it.
Sometimes there is a legitimate use of it, when you want to always
schedule an idle action (without intent to cancel or track it). That
makes more sense for g_idle_add() than it does for g_timeout_add(),
because a timeout really should be tracked and cancelled if necessary.
Add a wrapper to rename the legitimate uses. This way, we can avoid the
checkpatch.pl warnings, and can grep for the remaining illegitimate uses.
The numeric source IDs exist from a time before 2000, when there
was only one "GMainContext" singleton instance. Nowadays, the source
ID is only relative to one GMainContext, and you'd have to track
that association yourself. Als, g_source_remove() requires an additional
hash lookup, when you could simply track the GSource instance from the
start.
This API should not be used anymore. Operate on GSouce instances
direclty and use API like
nm_clear_g_source_inst()
nm_g_idle_add_source()
nm_g_idle_souce_new()
nm_g_source_attach()
g_source_attach
g_source_destroy
g_source_unref
etc.
Note that if you don't care about to ever remove a source again, like
scheduling an idle action that should not be cancelled, then
g_idle_add(callback, user_data);
is fine. It is only problematic to do something with those numeric IDs.
checkpatch.pl would also flag those uses, but these are just warnings
and in the few cases where such a warning is emitted wrongly, it's find
to ignore them.
Our new format gets enforced by clang-format, and we now only use
four space indentation, instead of tabs.
Adjust the checkpatch script to account for that.
Also, now there are probably no cases left where we want to see any
tabs in our sources. Complain about any tabs we find.
This is inspired by the existing systemd integration, with a few differences:
* This parses the WPAD option, which systemd requested, but did not use.
* We hook into the DAD handling, only making use of the configured address
once DAD has completed successfully, and declining the lease if it fails.
There are still many areas of possible improvement. In particular, we need
to ensure the parsing of all options are compliant, as n-dhcp4 treats all
options as opaque, unlike sd-dhcp4. We probably also need to look at how
to handle failures and retries (in particular if we decline a lease).
We need to query the current MTU at client startu, as well as the hardware
broadcast address. Both these are provided by the kernel over netlink, so
it should simply be a matter of hooking that up with NM's netlink layer.
Contribution under LGPL2.0+, in addition to stated licenses.
In continations (that use spaces for alignment), don't allow the number
of leading tabs to change. Previously only removal of tabs was
disallowed, but addition doesn't make sense either, as only spaces
should be used for further alignemnt.
This catches situations like this:
|<-tab->all_work_and_no_play (makes,
|<-tab-> jack,
|<-tab-><-tab-> a dull boy);
It doesn't have to be at the end of line, there may be more words
following.
Fixes: d66a1ace23 ('contrib/checkpatch: avoid command injection in checkpatch.pl script')
The capture variables, $1, etc, are not valid unless the match
succeeded, and they're not cleared, either.
$ git checkout -B C origin/master && \
echo XXXXX > f.txt && \
git add f.txt && \
git commit -m 'this commit does something()'
Branch 'C' set up to track remote branch 'master' from 'origin'.
Reset branch 'C'
Your branch is up to date with 'origin/master'.
sh: -c: line 0: syntax error near unexpected token `('
sh: -c: line 0: `git log --abbrev=12 --pretty=format:"%h ('%s')" -1 does something() 2>/dev/null'
>>> VALIDATE "a169a98e14 this commit does something()"
(commit message):4: Commit 'does something()' does not seem to exist:
> Subject: [PATCH] this commit does something()
(commit message):4: Refer to the commit id properly: :
> Subject: [PATCH] this commit does something()
The patch does not validate.
This allows us to add a file "TODO.txt" in the top level directory.
This file is not intended to be merged to master, but keep track of
stuff that is still to do before merging a branch.
Let checkpatch.pl warn about the presence of such a file.
We have a few source code tags like "TODO" and "FIXME".
"XXX" is not intended to be merged, it is for marking
places in code while still working on it.
This takes current HEAD branch, and finds all the commits what
are not on master or one of the nm-1-* branches, and runs
checkpatch.pl on each.
The use is to run checkpatch.pl on all patches of a feature
branch.
Don't use the integer type before signed/unsigned, but the
other way around. That is,
unsigned long var;
instead of
long unsigned var;
Also, just use "unsigned" instead of "unsigned int".
Tabs are not only wrong after a space, they are always
wrong if they don't appear at the beginning of a line.
That would happen usually, when trying to align multiple
lines like
enum {
VALUE1 = 1;
OTHER_VALUE = 2;
};
When doing that, the alignment will only be correct, if the
reader later uses the same tab-width. Note that in NetworkManager
we recommend the tab-width to be 4 characters, but with our "smart
tab" indentation style, it wouldn't actually matter and the reader
is free to choose any other tab-width -- as long as we don't use
non-leading tabs.
Don't allow non-leading tabs.
We should not use glib typedefs for basic C types char, short, int,
long, float or double. We commonly do not use them, so enforce
consistency.
That is not true for typedefs like guint, which we commonly use
because it's shorter typing than "unsigned int" (or "int unsigned"
or "unsigned"). Whether or not to use guint is left undecided at this
point.
A naive code compliance checker. Invoke directly:
contrib/scripts/checkpatch.pl 0001-switch-comments-to-klingon.patch
contrib/scripts/checkpatch.pl hello.[ch] world.c
Use from a commit hook:
echo 'git format-patch --stdout -1 |contrib/scripts/checkpatch.pl || :>' \
>.git/hooks/post-commit
Or view the documentation with "perldoc contrib/scripts/checkpatch.pl"
