Re: [PATCH] libnm: Warning if private data is NULL

["Shih-Yuan Lee (FourDollars)" <sylee canonical com>]

On Fri, May 6, 2016 at 7:20 PM, Thomas Haller <thaller redhat com> wrote:

On Fri, 2016-05-06 at 14:01 +0800, Shih-Yuan Lee (FourDollars) wrote:
> Check the priv pointer before loop traverse.
> Warning this instead of leaving the program crash directly.
> ---
>  libnm/nm-manager.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/libnm/nm-manager.c b/libnm/nm-manager.c
> index a6c1f3f..7d86a51 100644
> --- a/libnm/nm-manager.c
> +++ b/libnm/nm-manager.c
> @@ -816,6 +816,11 @@ recheck_pending_activations (NMManager *self)
>       const GPtrArray *devices;
>       NMDevice *device;
>  
> +     if (!priv) {
> +             g_warning ("%s: private data should not be NULL",
> __func__);
> +             return;
> +     }
> +
>       /* For each pending activation, look for an active
> connection that has the
>        * pending activation's object path, where the active
> connection and its
>        * device have both updated their properties to point to
> each other, and


Hi Shih-Yuan,


thank you for your patch.


The problem is not really that @priv is NULL, but that @self is not
pointing to a valid object. I think a better patch would be:


 recheck_pending_activations (NMManager *self)
 {
-    NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
+    NMManagerPrivate *priv;
     GSList *iter, *next;
     NMActiveConnection *candidate;
     const GPtrArray *devices;
     NMDevice *device;
 
+    g_return_if_fail (NM_IS_MANAGER (self));
+
+    priv = NM_MANAGER_GET_PRIVATE (self);
+


to reduce the use of the dangling @self pointer to a minimal.


Using g_return_if_fail() is not wrong to catch a bug or to mititgate
a crash in face of a bug. But g_return_if_fail() cannot be the fix
for the bug.


Can you reproduce the issue easily?
Can you share a reproducer?


Thank you,
Thomas

Hi Thomas,

Sorry for the late reply.

I was very busy for other project in these days.

I can not reproduce this issue easily.

In fact, I don't know how to reproduce this issue.

But there are many reports from https://bugs.launchpad.net/ubuntu/+source/network-manager/+bug/1582301.

You may be not allowed to access the report directly so I attached it in the email.

I also opened https://bugzilla.gnome.org/show_bug.cgi?id=766554 for this issue.

Regards,

$4

-- 

Shih-Yuan Lee (FourDollars) | Software Engineer | Commercial Engineering - PC & Core Taipei | Ubuntu Engineering and Services | Canonical

Attachment: collection.png
Description: PNG image