Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.

Bug 1350201

Summary: nmcli failed to modify IPv4 routes metric on i686 system
Product: [Fedora] Fedora Reporter: Vladislav Grigoryev <vg.aetera>
Component: NetworkManagerAssignee: Beniamino Galvani <bgalvani>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 24CC: bgalvani, dcbw, fgiudici, lkundrak, psimerda, thaller, vg.aetera
Target Milestone: ---   
Target Release: ---   
Hardware: i686   
OS: Linux   
Whiteboard:
Fixed In Version: NetworkManager-1.2.4-2.fc24 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2016-08-19 19:49:56 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
[PATCH] cli: fix parsing of route metric on 32-bit archs
none
[PATCH v2] cli: fix parsing of route metric on 32-bit archs none

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.