[PATCH 2/2] dbus: expose interface globals via D-Bus properties

[Jouni Malinen]

On Mon, Oct 12, 2015 at 12:08:25PM -0500, Dan Williams wrote:
> Which warnings do you have enabled?  I should enable those too.  I have
> gcc 4.9.2 and default CFLAGS, so that means " -MMD -O2 -Wall -g" per
> Makefile.  Perhaps you're using gcc 5.x and it has enabled some new
> warning flags by default?

These are from gcc 4.8.4 and I have a number of -W* arguments in
CFLAGS.. I'm not sure which ones of these actually trigger the warnings,
but based on the warnings themselves, -Wmissing-field-initializers and
-Wsign-compare seem to be the ones. I guess this one in
wpa_supplicant/.config ended up enabling these:
CFLAGS += -Wall -Wextra -Wno-unused-parameter

> I increased the buffer allocation there at least once already for
> introspection, but I suppose it dependsa on how much you've compiled in
> with CONFIG_xxx=y.  We should fix that better there in the future,
> instead of just bumping the value.  Also, we should just allocate that
> once since it's never going to change.

Agreed, it would be nice to determine this automatically or alternative,
reallocate the buffer if needed. It is not sufficient to update just the
per-interface wpabuf_alloc(); also the encapsulating buffer needs to be
larger. And yes, I do have all build options that can affect this size
enabled.

> > so many memory leaks that I did not really want to spend any more time
> > on this..
> 
> Which ones?  The only memory allocated in this 2/2 patch is
> wpas_dbus_all_interface_properties which is allocated when the first
> wifi interface is added and then kept in-memory after that since it is
> constant and needed whenever another interface is added.  There's no
> point in allocating it per-interface since that would just waste memory.
> Inside that, we need to convert the wpa-supplicant property name to a
> D-Bus style one, so that requires another allocation but again this is
> constant and long-lived.

It looks like there is a large number of leaks from here:

1444683046.251012: [2]: /home/jm/Git/hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(wpas_dbus_register_interface+0x225) [0x58bd45]
1444683046.251120:      uscore_to_dbus() dbus/dbus_new.c:3409
1444683046.251327:      wpas_dbus_register_interface() dbus/dbus_new.c:3499

(os_zalloc() in uscore_to_dbus(); by the way, there is no code to verify
that this allocation succeeded, i.e., this can segfault..)

and one of this:

1444683046.802881: [2]: /home/jm/Git/hostap/tests/hwsim/../../wpa_supplicant/wpa_supplicant(wpas_dbus_register_interface+0x12d) [0x58bc4d]
1444683046.802940:      wpas_dbus_register_interface() dbus/dbus_new.c:3474


1444683046.803362: MEMLEAK: total 8142 bytes


> I'm now converting this global array to one that is allocated on dbus
> interface setup and freed when the global dbus interface is de-inited.
> That ensures that no memory is left allocated at exit, with the downside
> that the supplicant will now always allocate this memory, regardless of
> whether or not any interface is ever added.

I guess that's fine, but anyway, I will require all memory to be freed
when the process exits to make memory leak analysis a reasonable task.

-- 
Jouni Malinen                                            PGP id EFC895FA