Bug#811640: regarding dasher gcc6 ftbfs / 5.0 beta

[Andreas Henriksson]

Hello intrigeri!

Read your bug report (#835533) and spent a few seconds looking at
the gcc6 ftbfs (#811640). The current ftbfs looks trivial to resolve
bit apparently noone cares enough about the package.

> DasherAppSettings.cpp:398:14: error: cannot convert 'bool' to 'const gchar* {aka const char*}' in return
>        return false;
>               ^~~~~

Looking at the callers of the function the correct return should apparently
be "" here as callers checks for that or for strlen == 0.

> DasherAppSettings.cpp: In function 'void dasher_app_settings_launch_advanced(DasherAppSettings*)':
> DasherAppSettings.cpp:505:15: warning: ISO C++ forbids converting a string constant to 'gchar* {aka char*}' [-Wwrite-strings]
>    szArgs[0] = "gconf-editor";
>                ^~~~~~~~~~~~~~

This one is not so obvious to me. For "C++ correctness" you should apparently
change the declaration of szArgs to be const char* rather than char*, but
I think that'll not last long as szArgs is passed to g_spawn_async
which doesn't take a 'const' char*, only a char*. I'm not sure if there's
any reason g_spawn_async hasn't been constified, but I assume it mostly
that it was written with C in mind and this is no problem there...

My spontaneous idea for solution was thus to simply cast the assignments:

szArgs[0] = (char*)"gconf-editor";
szArgs[1] = (char*)"/apps/dasher4";

Since you mentioned there was upstream activity I looked at how they solved
it in:
https://github.com/ipomoena/dasher/blob/master/Src/Gtk2/DasherAppSettings.cpp#L81
(Not sure this is the correct upstream repository though.)

This does not look correct at all to me. Please note the strdup which will
need it to get freed at some point to not leak memory, then call to
g_spawn_async and given it's an async method I assume it won't really
do any work at all until we let the main loop run again, then *directly*
calls g_strfreev(szArgs);
Without investigating any deeper I think this must result in a use-after-free.

We need someone that's interested in dasher to maintain it properly.
Apparently upstream could use an extra pair of eyes to help keep them
safe. This is thus my attempt at suckering you, who have shown atleast
a bit of interest in dasher, to be that person.
If not, the quick route would probably just be to add a patch with
the above suggested solution (which I've verified fixes the build)
and do an NMU to get dasher back into testing.

Regards,
Andreas Henriksson