I think it's preferable to use nm_clear_g_free() instead of
g_clear_pointer(, g_free). The reasons are not very strong,
but I think it is overall preferable to have a shorthand for this
frequently used functionality.
sed 's/\<g_clear_pointer *(\([^;]*\), *\(g_free\) *)/nm_clear_g_free (\1)/g' $(git grep -l g_clear_pointer) -i
nl80211_alloc_msg() just allocates some memory, using glib's allocators.
Hence it cannot fail, and we don't need to check for that.
Drop the unnecessary %NULL checks.
(cherry picked from commit 990a7bee9d)
Usually we check the result of nla_nest_start(). Also, in most cases where this
function would return %NULL, it's an actual bug. That is, because our netlink
message is allocated with a large buffer, and in most cases we append there a well
known, small amount of data.
To make coverity happy, handle the case and assert.
(cherry picked from commit 243458836a)
We no longer add these. If you use Emacs, configure it yourself.
Also, due to our "smart-tab" usage the editor anyway does a subpar
job handling our tabs. However, on the upside every user can choose
whatever tab-width he/she prefers. If "smart-tabs" are used properly
(like we do), every tab-width will work.
No manual changes, just ran commands:
F=($(git grep -l -e '-\*-'))
sed '1 { /\/\* *-\*- *[mM]ode.*\*\/$/d }' -i "${F[@]}"
sed '1,4 { /^\(#\|--\|dnl\) *-\*- [mM]ode/d }' -i "${F[@]}"
Check remaining lines with:
git grep -e '-\*-'
The ultimate purpose of this is to cleanup our files and eventually use
SPDX license identifiers. For that, first get rid of the boilerplate lines.
- drop explicit MAX sizes like
static const struct nla_policy policy[IFLA_INET6_MAX+1] = {
The compiler will deduce that.
It saves redundant information (which is possibly wrong). Also,
the max define might be larger than we actually need it, so we
just waste a few bytes of static memory and do unnecesary steps
during validation.
Also, the compiler will catch bugs, if the array size of policy/tb
is too short for what we access later (-Warray-bounds).
- avoid redundant size specifiers like:
static const struct nla_policy policy[IFLA_INET6_MAX+1] = {
...
struct nlattr *tb[IFLA_INET6_MAX+1];
...
err = nla_parse_nested (tb, IFLA_INET6_MAX, attr, policy);
- use the nla_parse*_arr() macros that determine the maxtype
based on the argument types.
- move declaration of "static const struct nla_policy policy" variable
to the beginning, before auto variables.
- drop unneeded temporay error variables.
Correct the spelling across the *entire* tree, including translations,
comments, etc. It's easier that way.
Even the places where it's not exposed to the user, such as tests, so
that we learn how is it spelled correctly.
In the past, the headers "linux/if.h" and "net/if.h" were incompatible.
That means, we can either include one or the other, but not both.
This is fixed in the meantime, however the issue still exists when
building against older kernel/glibc.
That means, including one of these headers from a header file
is problematic. In particular if it's a header like "nm-platform.h",
which itself is dragged in by many other headers.
Avoid that by not including these headers from "platform.h", but instead
from the source files where needed (or possibly from less popular header
files).
Currently there is no problem. However, this allows an unknowing user to
include <net/if.h> at the same time with "nm-platform.h", which is easy
to get wrong.
- drop "goto error_label" in favor of returning right away.
At most places, there was no need to do any cleanup or
the cleanup is handled via nm_auto().
- adjust the return types of wireguard functions to return
a boolean success/failure, instead of some error code which
we didn't use.
- the change to _wireguard_get_link_properties() is intentional.
This was wrong previously, because in _wireguard_get_link_properties()
obj is always a newly created instance, and never has a genl
family ID set. This will be improved later.
GBytes makes more sense, because it's immutable.
Also, since at other places we use GBytes, having
different types is combersome and requires needless
conversions.
Also:
- avoid nm_utils_escape_ssid() instead of _nm_utils_ssid_to_string().
We use nm_utils_escape_ssid() when we want to log the SSID. However, it
does not escape newlines, which is bad.
- also no longer use nm_utils_same_ssid(). Since it no longer
treated trailing NUL special, it is not different from
g_bytes_equal().
- also, don't use nm_utils_ssid_to_utf8() for logging anymore.
For logging, _nm_utils_ssid_escape_utf8safe() is better because
it is loss-less escaping which can be unambigously reverted.
- in wake_on_wlan_restore(), if we decide that there is something
to restore, also clear priv->wowlan_restore by setting it to
IGNORE. That way, we are sure to only try resetting the value
once after setting it.
- from nm_platform_wifi_get_wake_on_wlan(), return IGNORE if
the value cannot be read. If we could not read the value
we should not restore NONE, but don't restore.
Passing ownership with a function call is confusing. Don't do that.
Since we have the cleanup attribute, it doesn't significantly
complicate the callers, as all they need to do is marking the
@msg variable to free the message when going out of scope.
That results in the function that allocates the message also being
responsible to free it.
The callers expect _nl80211_send_and_recv() to free @msg.
This was broken by the previous commit, which wrongly removed
the nm_auto_nlmsg cleanup attribute.
Fix the compiler warning differently.
Fixes: d7108d9362
Glib is not out of memory safe, meaning it always aborts the program
when an allocation fails. It is not possible to meaningfully handle
out of memory when using glib.
Replace all allocation functions for netlink message with their glib
counter part and remove the NULL checks.
With libnl3, each socket has it's own callback structure.
One would often take that callback structure, clone it, modify it
and invoke a receive operation with it.
We don't need this complexity. We got rid of all default handlers,
hence, by default all callbacks are unset.
The only callbacks that are set, are those that we specify immediately
before invoking the receive operation. Just pass the callback structure
at that point.
Also, no more ref-counting, and cloning of the callback structure. It is
so simple, just stack allocate one if you need it.
