segfault on string compare

[chipschap]

When initially running audio-recorder I was getting segfaults. I rebuilt from latest source, ran with gdb and found in utility.c around line 1049

gint str_compare(const gchar *s1, const gchar *s2, gboolean case_insensitive) {
    if (*s1 == '\0' && *s2 == '\0') {
        // Equals
        return 0;

etc.

There is no check here for passing null pointers and somehow one got passed. So my temporary fix was this. I know it's not the right fix (s1 and s2 should be first tested outside of the if stmt) but it got around the problem.

gint str_compare(const gchar *s1, const gchar *s2, gboolean case_insensitive) {
    if (s1 && (*s1 == '\0') && s2 && (*s2 == '\0')) {
        // Equals
        return 0;

etc.

My environment is Arch Linux (Antergos flavor).

[Roy Wellington (cactus-hugged)]

I'm also an Arch user, and this program also segfaults for me at startup. The segfault is the same — a crash in str_compare due to passing a null pointer. I can provide a more complete backtrace, however:

(gdb) bt
#0 0x000055555557744a in str_compare (s1=0x0, s2=0x555555582ea3 "gnome-music", case_insensitive=1) at utility.c:1050
#1 0x0000555555566ddb in mpris2_detect_players () at dbus-mpris2.c:1168
#2 0x0000555555567b02 in dbus_player_get_player_list () at dbus-player.c:494
#3 0x0000555555563969 in audio_sources_load_device_list () at audio-sources.c:561
#4 0x0000555555563ef4 in audio_source_fill_combo (combo=0x5555559fa210) at audio-sources.c:669
#5 0x000055555557ff49 in win_create_window () at main.c:1211
#6 0x0000555555580b35 in main (argc=1, argv=0x7fffffffe028) at main.c:1508
(gdb) up
#1 0x0000555555566ddb in mpris2_detect_players () at dbus-mpris2.c:1168
1168 if (str_compare(player->desktop_file, "gnome-music", TRUE) == 0) {
(gdb) p player
$1 = (MediaPlayerRec *) 0x555555a69780
(gdb) p player->desktop_file
$2 = (gchar *) 0x0

That's shortly after this section of code grabs the Desktop entry for the media player:

// Get player's desktop file.
// Ref: https://specifications.freedesktop.org/mpris-spec/latest/Media_Player.html
player->desktop_file = mpris2_get_property_str(player, "DesktopEntry");

The linked specification at that URL notes that that property is *optional*, which is why I presume we get a null here; the next bit of code, a FIXME, seems to presume it does. The FIXME states that gnome-music reports the wrong value, but that doesn't seem to apply here. (That comment seems to refer to a gnome-music where the DesktopEntry property is "gnome-music" instead of null.) Nonetheless, the check for it is what kills us:

if (str_compare(player->desktop_file, "gnome-music", TRUE) == 0) {

Either str_compare can be adjusted to handle null, or one can change the line to:

if (player->desktop-file && str_compare[…]) {

to avoid it. The remaining uses seem to handle nulls.

Nonetheless, even after fixing this, I don't seem to get a working audio-recorder. It hangs, and I can't force a window or icon w/ -w 1 -i 1, nor can I seem to send commands to it.

[moma (osmoma)]

Hello chipschap and Roy Wellington,

Many thanks reporting your finding.

I will now update the code with your fix.

And sorry for my delayed answer.
I have been away from development some time.

The code needs a re-design + re-write, so do I.

Thanks
  Osmo Antero e Bica
  Portugal

[moma (osmoma)]

Please see:
https://bazaar.launchpad.net/~audio-recorder/audio-recorder/trunk/files

Rev. 1441 and 1442.

All good.