After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 739861 - [review] lr/crashes: Failure in test suite and dhcp on i686
[review] lr/crashes: Failure in test suite and dhcp on i686
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-11-09 22:23 UTC by Lubomir Rintel
Modified: 2014-12-01 11:05 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Lubomir Rintel 2014-11-09 22:23:43 UTC
Running the current master was not very successful for me.

Here's a branch with two small and unrelated fixes which restored my inner peace:
http://cgit.freedesktop.org/NetworkManager/NetworkManager/log/?h=lr/crashes
Comment 1 Thomas Haller 2014-11-10 10:18:22 UTC
>> dhcp-manager: Keep size of PID consistent

Indeed GPid is a typedef for int, and most likely all platforms we ever support will have pid_t <= int. And glib already has int >= int32... hence it's correct.

Indeed we can commit to pid_t <= int32 <= int.



You could also just fix it with:

-    g_signal_emit (self, signals[EVENT], 0, iface, pid, options, reason, &handled);
+    g_signal_emit (self, signals[EVENT], 0, iface, (gint64) pid, options, reason, &handled);


But yeah, I'm Ok with assuming pid_t type is gint or gint32.



while at it, I'd also change:

-    pid = (gint32) nm_utils_ascii_str_to_int64 (pid_str, 10, 0, G_MAXINT32, -1);
-    if (pid == -1 || pid != (GPid) pid) {
+    pid = nm_utils_ascii_str_to_int64 (pid_str, 10, 2, G_MAXINT32, -1);
+    if (pid == -1) {

nm_utils_ascii_str_to_int64() already does full range checking, and will not ever return a value out of range (except @fallback).




I think the commit messages don't really describe the problem, but never mind :) .



Rest looks good
Comment 2 Lubomir Rintel 2014-11-10 12:32:46 UTC
Fixed in master.