Bug 1350201 - nmcli failed to modify IPv4 routes metric on i686 system
Summary: nmcli failed to modify IPv4 routes metric on i686 system
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: NetworkManager
Version: 24
Hardware: i686
OS: Linux
unspecified
unspecified
Target Milestone: ---
Assignee: Beniamino Galvani
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-06-26 12:15 UTC by Vladislav Grigoryev
Modified: 2016-08-19 19:49 UTC (History)
7 users (show)

Fixed In Version: NetworkManager-1.2.4-2.fc24
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-08-19 19:49:56 UTC
Type: Bug


Attachments (Terms of Use)
[PATCH] cli: fix parsing of route metric on 32-bit archs (2.65 KB, patch)
2016-06-27 13:51 UTC, Beniamino Galvani
no flags Details | Diff
[PATCH v2] cli: fix parsing of route metric on 32-bit archs (2.65 KB, patch)
2016-06-30 10:01 UTC, Beniamino Galvani
no flags Details | Diff

Description Vladislav Grigoryev 2016-06-26 12:15:53 UTC
Description of problem:
$ nmcli connection add type ethernet con-name test ifname "*"
Connection 'test' (4029438a-eedd-4606-be32-6f9d2da6bea7) successfully added.
$ nmcli connection modify test ipv4.routes "10.10.10.0/24 192.168.1.254 10"
Error: failed to modify ipv4.routes: invalid metric '10'.

Version-Release number of selected component (if applicable):
NetworkManager-1.2.2-2.fc24.i686

How reproducible:
Always.

Steps to Reproduce:
1. Use i686.
2. nmcli connection add type ethernet con-name test ifname "*"
3. nmcli connection modify test ipv4.routes "10.10.10.0/24 192.168.1.254 10"

Actual results:
nmcli failed to modify IPv4 routes metric on i686 system.

Comment 1 Beniamino Galvani 2016-06-27 13:51:23 UTC
Created attachment 1172885 [details]
[PATCH] cli: fix parsing of route metric on 32-bit archs

Untested patch.

Comment 2 Thomas Haller 2016-06-30 08:08:45 UTC
(In reply to Beniamino Galvani from comment #1)
> Created attachment 1172885 [details]
> [PATCH] cli: fix parsing of route metric on 32-bit archs
> 
> Untested patch.

I think you have to initialize @metric, otherwise (depending on compiler optimiation flags) it may emit a warning about uninitialized variable.


and in "(gint64) (metric_valid ? metric : -1)" inside the ternary operator we have types "? unsigned long : signed". This will be promoted to signed long (I guess). Which is actually fine, but it still makes me hesitate for 5 seconds to reason whether it is correct. I'd avoid that by doing the cast inside the ternary operator.


Can you not just avoid that by doing:

  gint64 metric = -1;
  ...
      if (!nmc_string_to_uint (third, TRUE, 0, G_MAXUINT32, &tmp_ulong)) {
      }
      metric = tmp_ulong;





(btw. how hard can it be to get a string-to-int function right :) ? We should eventually replace this with _nm_utils_ascii_str_to_int64()).


rest, lgtm

Comment 3 Beniamino Galvani 2016-06-30 10:01:41 UTC
Created attachment 1174454 [details]
[PATCH v2] cli: fix parsing of route metric on 32-bit archs

(In reply to Thomas Haller from comment #2)

> Can you not just avoid that by doing:

Changed, thanks.

Comment 4 Francesco Giudici 2016-06-30 10:29:55 UTC
lgtm

Comment 5 Thomas Haller 2016-06-30 11:25:48 UTC
v2 lgtm

Comment 7 Fedora Update System 2016-08-05 07:52:47 UTC
NetworkManager-1.2.4-1.fc24 network-manager-applet-1.2.4-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-f739ece3cf

Comment 8 Fedora Update System 2016-08-05 21:21:33 UTC
NetworkManager-1.2.4-1.fc24, network-manager-applet-1.2.4-1.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-f739ece3cf

Comment 9 Fedora Update System 2016-08-18 16:38:52 UTC
NetworkManager-1.2.4-2.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-fade485364

Comment 10 Fedora Update System 2016-08-19 00:56:46 UTC
NetworkManager-1.2.4-2.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-fade485364

Comment 11 Fedora Update System 2016-08-19 10:29:01 UTC
network-manager-applet-1.2.4-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-f739ece3cf

Comment 12 Fedora Update System 2016-08-19 19:49:31 UTC
NetworkManager-1.2.4-2.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.