<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/NetworkManager.git/src/libnm-client-impl/tests, branch lr/asserts</title>
<subtitle>gitlab.freedesktop.org: NetworkManager/NetworkManager.git
</subtitle>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/'/>
<entry>
<title>libnm/tests: test that nm_vpn_service_plugin_read_vpn_details() does not consume "QUIT" command</title>
<updated>2022-03-28T08:40:00+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2022-03-28T08:37:08+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=b1b1ee8cc41af4f57902560f6769daf15852311e'/>
<id>b1b1ee8cc41af4f57902560f6769daf15852311e</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>libnm: handle NUL characters in nm_vpn_service_plugin_read_vpn_details() and fix test</title>
<updated>2022-03-28T08:36:05+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2022-03-28T07:40:22+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=6235815248314c0bd3deb485692881620a859cf9'/>
<id>6235815248314c0bd3deb485692881620a859cf9</id>
<content type='text'>
We expect to read NUL terminated strings. Upon NUL, we should do
something. Treat it as a line break.

Fixes: 8ae9cf4698b4 ('Revert "libnm: buffer reads in nm_vpn_service_plugin_read_vpn_details()"')
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
We expect to read NUL terminated strings. Upon NUL, we should do
something. Treat it as a line break.

Fixes: 8ae9cf4698b4 ('Revert "libnm: buffer reads in nm_vpn_service_plugin_read_vpn_details()"')
</pre>
</div>
</content>
</entry>
<entry>
<title>all: suppress wrong gcc-12 warning "-Wdangling-pointer"</title>
<updated>2022-02-21T18:50:52+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2022-02-18T19:34:17+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=dab2ee8ac5c025cc8f3dcb0f0aae7bcecab961e4'/>
<id>dab2ee8ac5c025cc8f3dcb0f0aae7bcecab961e4</id>
<content type='text'>
gcc-12.0.1-0.8.fc36 is annoying with false positives.
It's related to g_error() and its `for(;;) ;`.

For example:

    ../src/libnm-glib-aux/nm-shared-utils.c: In function 'nm_utils_parse_inaddr_bin_full':
    ../src/libnm-glib-aux/nm-shared-utils.c:1145:26: error: dangling pointer to 'error' may be used [-Werror=dangling-pointer=]
     1145 |                     error-&gt;message);
          |                          ^~
    /usr/include/glib-2.0/glib/gmessages.h:343:32: note: in definition of macro 'g_error'
      343 |                                __VA_ARGS__);         \
          |                                ^~~~~~~~~~~
    ../src/libnm-glib-aux/nm-shared-utils.c:1133:31: note: 'error' declared here
     1133 |         gs_free_error GError *error = NULL;
          |                               ^~~~~
    /usr/include/glib-2.0/glib/gmessages.h:341:25: error: dangling pointer to 'addrbin' may be used [-Werror=dangling-pointer=]
      341 |                         g_log (G_LOG_DOMAIN,         \
          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      342 |                                G_LOG_LEVEL_ERROR,    \
          |                                ~~~~~~~~~~~~~~~~~~~~~~~
      343 |                                __VA_ARGS__);         \
          |                                ~~~~~~~~~~~~
    ../src/libnm-glib-aux/nm-shared-utils.c:1141:13: note: in expansion of macro 'g_error'
     1141 |             g_error("unexpected assertion failure: could parse \"%s\" as %s, but not accepted by "
          |             ^~~~~~~
    ../src/libnm-glib-aux/nm-shared-utils.c:1112:14: note: 'addrbin' declared here
     1112 |     NMIPAddr addrbin;
          |              ^~~~~~~

I think the warning could potentially be useful and prevent real bugs.
So don't disable it altogether, but go through the effort to suppress it
at the places where it currently happens.

Note that NM_PRAGMA_WARNING_DISABLE_DANGLING_POINTER macro only expands
to suppressing the warning with __GNUC__ equal to 12. The purpose is to
only suppress the warning where we know we want to. Hopefully other gcc
versions don't have this problem.

I guess, we could also write a NM_COMPILER_WARNING() check in
"m4/compiler_options.m4", to disable the warning if we detect it. But
that seems too cumbersome.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
gcc-12.0.1-0.8.fc36 is annoying with false positives.
It's related to g_error() and its `for(;;) ;`.

For example:

    ../src/libnm-glib-aux/nm-shared-utils.c: In function 'nm_utils_parse_inaddr_bin_full':
    ../src/libnm-glib-aux/nm-shared-utils.c:1145:26: error: dangling pointer to 'error' may be used [-Werror=dangling-pointer=]
     1145 |                     error-&gt;message);
          |                          ^~
    /usr/include/glib-2.0/glib/gmessages.h:343:32: note: in definition of macro 'g_error'
      343 |                                __VA_ARGS__);         \
          |                                ^~~~~~~~~~~
    ../src/libnm-glib-aux/nm-shared-utils.c:1133:31: note: 'error' declared here
     1133 |         gs_free_error GError *error = NULL;
          |                               ^~~~~
    /usr/include/glib-2.0/glib/gmessages.h:341:25: error: dangling pointer to 'addrbin' may be used [-Werror=dangling-pointer=]
      341 |                         g_log (G_LOG_DOMAIN,         \
          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      342 |                                G_LOG_LEVEL_ERROR,    \
          |                                ~~~~~~~~~~~~~~~~~~~~~~~
      343 |                                __VA_ARGS__);         \
          |                                ~~~~~~~~~~~~
    ../src/libnm-glib-aux/nm-shared-utils.c:1141:13: note: in expansion of macro 'g_error'
     1141 |             g_error("unexpected assertion failure: could parse \"%s\" as %s, but not accepted by "
          |             ^~~~~~~
    ../src/libnm-glib-aux/nm-shared-utils.c:1112:14: note: 'addrbin' declared here
     1112 |     NMIPAddr addrbin;
          |              ^~~~~~~

I think the warning could potentially be useful and prevent real bugs.
So don't disable it altogether, but go through the effort to suppress it
at the places where it currently happens.

Note that NM_PRAGMA_WARNING_DISABLE_DANGLING_POINTER macro only expands
to suppressing the warning with __GNUC__ equal to 12. The purpose is to
only suppress the warning where we know we want to. Hopefully other gcc
versions don't have this problem.

I guess, we could also write a NM_COMPILER_WARNING() check in
"m4/compiler_options.m4", to disable the warning if we detect it. But
that seems too cumbersome.
</pre>
</div>
</content>
</entry>
<entry>
<title>libnm/tests: enable check for dangling pointer in test_activate_virtual()</title>
<updated>2022-01-28T10:01:53+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2022-01-27T20:41:43+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=125b1f9a9a934a578af01ffd98eaf68e92c5cb5a'/>
<id>125b1f9a9a934a578af01ffd98eaf68e92c5cb5a</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>Revert "libnm: fix dangling pointer in public API while destructing NMClient"</title>
<updated>2022-01-25T20:40:38+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2022-01-25T20:38:27+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=62b2aa85e875d9c842f878cac896dd1fa7c5d988'/>
<id>62b2aa85e875d9c842f878cac896dd1fa7c5d988</id>
<content type='text'>
This breaks test @nmcli_monitor. With this patch, `nmcli monitor` no
longer prints "There's no primary connection". Need to investigate why.
For now, revert the patch.

This reverts commit 2afecaf908401296a2642fdd1d73a76b9c3fa11a.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This breaks test @nmcli_monitor. With this patch, `nmcli monitor` no
longer prints "There's no primary connection". Need to investigate why.
For now, revert the patch.

This reverts commit 2afecaf908401296a2642fdd1d73a76b9c3fa11a.
</pre>
</div>
</content>
</entry>
<entry>
<title>libnm/tests: fix race in test test_activate_virtual()</title>
<updated>2022-01-21T12:44:01+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2022-01-21T12:43:54+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=26c43e4bccfeedc0028606794139c1749fdaf49f'/>
<id>26c43e4bccfeedc0028606794139c1749fdaf49f</id>
<content type='text'>
  ERROR: src/libnm-client-impl/tests/test-nm-client - Bail out! nm:ERROR:src/libnm-client-impl/tests/test-nm-client.c:807:_dev_eth0_1_state_changed_cb: assertion failed (old_state == NM_DEVICE_STATE_PREPARE): (100 == 40)

Fixes: bc9aa72c8853 ('libnm/tests: add unit test for checking dangling pointer in libnm')
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
  ERROR: src/libnm-client-impl/tests/test-nm-client - Bail out! nm:ERROR:src/libnm-client-impl/tests/test-nm-client.c:807:_dev_eth0_1_state_changed_cb: assertion failed (old_state == NM_DEVICE_STATE_PREPARE): (100 == 40)

Fixes: bc9aa72c8853 ('libnm/tests: add unit test for checking dangling pointer in libnm')
</pre>
</div>
</content>
</entry>
<entry>
<title>libnm: fix dangling pointer in public API while destructing NMClient</title>
<updated>2022-01-21T11:09:45+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2022-01-18T07:41:43+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=2afecaf908401296a2642fdd1d73a76b9c3fa11a'/>
<id>2afecaf908401296a2642fdd1d73a76b9c3fa11a</id>
<content type='text'>
While (and after) NMClient gets destroyed, nm_device_get_active_connection()
gives a dangling pointer. That can lead to a crash. This probably
affects all NMLDBusPropertyO type properties.

It's not clear how to fix that best. Usually, NMClient does updates in
two phases, first it processes the D-Bus events and tracks internal
data, then it emits all GObject signals and notifications.

When an object gets removed from the NMClient cache, then the second
phase is not fully processed, because the object is already removed
from the cache. Thus, the property was not properly cleared leaving
a dangling pointer.

A simple fix is to always clear the pointer during the first phase. Note that
effectively we do the same also for NMLDBusPropertyAO (by clearing the
"pr_ao-&gt;arr"), so at least this is consistent.

Somehow it seems that we should make sure that the "second" phase gets
full processed in this case too. But it's complicated, and it's not
clear how to do that. So this solution seems fine.

https://bugzilla.redhat.com/show_bug.cgi?id=2039331
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/896
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
While (and after) NMClient gets destroyed, nm_device_get_active_connection()
gives a dangling pointer. That can lead to a crash. This probably
affects all NMLDBusPropertyO type properties.

It's not clear how to fix that best. Usually, NMClient does updates in
two phases, first it processes the D-Bus events and tracks internal
data, then it emits all GObject signals and notifications.

When an object gets removed from the NMClient cache, then the second
phase is not fully processed, because the object is already removed
from the cache. Thus, the property was not properly cleared leaving
a dangling pointer.

A simple fix is to always clear the pointer during the first phase. Note that
effectively we do the same also for NMLDBusPropertyAO (by clearing the
"pr_ao-&gt;arr"), so at least this is consistent.

Somehow it seems that we should make sure that the "second" phase gets
full processed in this case too. But it's complicated, and it's not
clear how to do that. So this solution seems fine.

https://bugzilla.redhat.com/show_bug.cgi?id=2039331
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/896
</pre>
</div>
</content>
</entry>
<entry>
<title>libnm/tests: add unit test for checking dangling pointer in libnm</title>
<updated>2022-01-21T11:08:01+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2022-01-17T16:54:58+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=bc9aa72c885386d8f0942810e010d6ba6a03a4cd'/>
<id>bc9aa72c885386d8f0942810e010d6ba6a03a4cd</id>
<content type='text'>
When destroying NMClient, nm_device_get_active_connection() still
return dangling pointers. Add a unit test for that bug.

Obviously, the bug currently exists, so the relevant code is commented
out.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When destroying NMClient, nm_device_get_active_connection() still
return dangling pointers. Add a unit test for that bug.

Obviously, the bug currently exists, so the relevant code is commented
out.
</pre>
</div>
</content>
</entry>
<entry>
<title>format: reformat source tree with clang-format 13.0</title>
<updated>2021-11-29T09:31:09+00:00</updated>
<author>
<name>Thomas Haller</name>
<email>thaller@redhat.com</email>
</author>
<published>2021-11-09T12:28:54+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=615221a99cdd5059b2632cba923bc04c92165dbc'/>
<id>615221a99cdd5059b2632cba923bc04c92165dbc</id>
<content type='text'>
We use clang-format for automatic formatting of our source files.
Since clang-format is actively maintained software, the actual
formatting depends on the used version of clang-format. That is
unfortunate and painful, but really unavoidable unless clang-format
would be strictly bug-compatible.

So the version that we must use is from the current Fedora release, which
is also tested by our gitlab-ci. Previously, we were using Fedora 34 with
clang-tools-extra-12.0.1-1.fc34.x86_64.

As Fedora 35 comes along, we need to update our formatting as Fedora 35
comes with version "13.0.0~rc1-1.fc35".
An alternative would be to freeze on version 12, but that has different
problems (like, it's cumbersome to rebuild clang 12 on Fedora 35 and it
would be cumbersome for our developers which are on Fedora 35 to use a
clang that they cannot easily install).

The (differently painful) solution is to reformat from time to time, as we
switch to a new Fedora (and thus clang) version.
Usually we would expect that such a reformatting brings minor changes.
But this time, the changes are huge. That is mentioned in the release
notes [1] as

  Makes PointerAligment: Right working with AlignConsecutiveDeclarations. (Fixes https://llvm.org/PR27353)

[1] https://releases.llvm.org/13.0.0/tools/clang/docs/ReleaseNotes.html#clang-format
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
We use clang-format for automatic formatting of our source files.
Since clang-format is actively maintained software, the actual
formatting depends on the used version of clang-format. That is
unfortunate and painful, but really unavoidable unless clang-format
would be strictly bug-compatible.

So the version that we must use is from the current Fedora release, which
is also tested by our gitlab-ci. Previously, we were using Fedora 34 with
clang-tools-extra-12.0.1-1.fc34.x86_64.

As Fedora 35 comes along, we need to update our formatting as Fedora 35
comes with version "13.0.0~rc1-1.fc35".
An alternative would be to freeze on version 12, but that has different
problems (like, it's cumbersome to rebuild clang 12 on Fedora 35 and it
would be cumbersome for our developers which are on Fedora 35 to use a
clang that they cannot easily install).

The (differently painful) solution is to reformat from time to time, as we
switch to a new Fedora (and thus clang) version.
Usually we would expect that such a reformatting brings minor changes.
But this time, the changes are huge. That is mentioned in the release
notes [1] as

  Makes PointerAligment: Right working with AlignConsecutiveDeclarations. (Fixes https://llvm.org/PR27353)

[1] https://releases.llvm.org/13.0.0/tools/clang/docs/ReleaseNotes.html#clang-format
</pre>
</div>
</content>
</entry>
<entry>
<title>libnm: introduce nm_device_get_ports() to NMDevice</title>
<updated>2021-10-11T07:38:24+00:00</updated>
<author>
<name>Fernando Fernandez Mancera</name>
<email>ffmancera@riseup.net</email>
</author>
<published>2021-10-06T14:12:58+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/NetworkManager.git/commit/?id=74dfc86aa451985283abb7e617aadd9a18aef88f'/>
<id>74dfc86aa451985283abb7e617aadd9a18aef88f</id>
<content type='text'>
This patch is introducing a "ports" property to NMDevice. In addition it
is introducing nm_device_get_ports() and deprecating
nm_device_bond_get_slaves(), nm_device_bridge_get_slaves(),
nm_device_ovs_bridge_get_slaves(), nm_device_ovs_interface_get_slaves()
and nm_device_team_get_slaves().

Signed-off-by: Fernando Fernandez Mancera &lt;ffmancera@riseup.net&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This patch is introducing a "ports" property to NMDevice. In addition it
is introducing nm_device_get_ports() and deprecating
nm_device_bond_get_slaves(), nm_device_bridge_get_slaves(),
nm_device_ovs_bridge_get_slaves(), nm_device_ovs_interface_get_slaves()
and nm_device_team_get_slaves().

Signed-off-by: Fernando Fernandez Mancera &lt;ffmancera@riseup.net&gt;
</pre>
</div>
</content>
</entry>
</feed>
