Re: [PATCH] libnm: Check the priv pointer before loop traverse.

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

I see. Thx for your review.

On Thu, May 5, 2016 at 4:11 PM, Thomas Haller <thaller redhat com> wrote:

On Wed, 2016-05-04 at 16:09 +0800, Shih-Yuan Lee (FourDollars) wrote:
> ---
>  libnm/nm-manager.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libnm/nm-manager.c b/libnm/nm-manager.c
> index a6c1f3f..9316fdb 100644
> --- a/libnm/nm-manager.c
> +++ b/libnm/nm-manager.c
> @@ -816,6 +816,8 @@ recheck_pending_activations (NMManager *self)
>       const GPtrArray *devices;
>       NMDevice *device;
>  
> +     g_return_if_fail (priv);
> +
>       /* 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,


This patch is not correct for two reasons:

- g_return_if_fail() and similar macros are assertions. They must never
be used for valid control flow.

- @priv at this place can only be NULL/invalid, if @self was
NULL/invalid. From the backtrace you see, that @self is not NULL. You
canot inspect a dangling, invalid pointer to decide that it is invalid.

The patch might accidentally avoid the crash, but it doesn't fix the
bug.


Thomas

--

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