Re: dbusbind.c SIGSEGV fix and minor cleanup

[David Hansen]

On Fri, 21 Mar 2008 17:27:07 +0100 Michael Albinus wrote:

> David Hansen <address@hidden> writes:
>
>> Hello,
>
> Hi,
>
>> emacs crashes (in `strcpy') when receiving dbus messages with some empty
>> content.
>>
>> The attached patch fixes this.  Also there is no need to copy the
>> strings.  This is already be done by `build_string'.
>
> One could argue, that in case interface or member is NULL, this could be
> regarded as wildcards, and all registered functions which match

At least with my dbus installation there is still an error coming to the actual
connection.  This empty message is hard to reproduce as it seem to happen more
or less randomly.  In case you wonder how I found out:

I debugged a program with gud that crashed after receiving a dbus message from
the same emacs process.  In some cases emacs just died with it.

> One minor point: before "RETURN_UNGCPRO (Qnil)" you might call
> "dbus_message_unref (dmessage)" for freeing dmessage. Or you simply jump
> to the end, where it is done.

Thanks for reviewing and pointing that out.

> Please commit the patch, or (in case you have no write access in CVS)
> send the ChangeLog entry; I'll do it then.

2008-03-22  David Hansen  <address@hidden>

dbusbind.c: (xd_read_message): Removed extra copying of message strings.  Check
for NULL `interface' or `member'.

*** dbusbind.c.~1.22.~  2008-02-17 01:40:28.000000000 +0100
--- dbusbind.c  2008-03-22 09:12:04.000000000 +0100
***************
*** 1140,1149 ****
    DBusMessageIter iter;
    unsigned int dtype;
    int mtype;
!   char uname[DBUS_MAXIMUM_NAME_LENGTH];
!   char path[DBUS_MAXIMUM_MATCH_RULE_LENGTH]; /* Unlimited in D-Bus spec.  */
!   char interface[DBUS_MAXIMUM_NAME_LENGTH];
!   char member[DBUS_MAXIMUM_NAME_LENGTH];
  
    /* Open a connection to the bus.  */
    connection = xd_initialize (bus);
--- 1140,1146 ----
    DBusMessageIter iter;
    unsigned int dtype;
    int mtype;
!   const char *uname, *path, *interface, *member;
  
    /* Open a connection to the bus.  */
    connection = xd_initialize (bus);
***************
*** 1175,1185 ****
  
    /* Read message type, unique name, object path, interface and member
       from the message.  */
!   mtype =            dbus_message_get_type (dmessage);
!   strcpy (uname,     dbus_message_get_sender (dmessage));
!   strcpy (path,      dbus_message_get_path (dmessage));
!   strcpy (interface, dbus_message_get_interface (dmessage));
!   strcpy (member,    dbus_message_get_member (dmessage));
  
    XD_DEBUG_MESSAGE ("Event received: %d %s %s %s %s %s",
                    mtype, uname, path, interface, member,
--- 1172,1186 ----
  
    /* Read message type, unique name, object path, interface and member
       from the message.  */
!   mtype     = dbus_message_get_type (dmessage);
!   uname     = dbus_message_get_sender (dmessage);
!   path      = dbus_message_get_path (dmessage);
!   interface = dbus_message_get_interface (dmessage);
!   member    = dbus_message_get_member (dmessage);
! 
!   /* dbus-registered-functions-table requires non nil interface and member. */
!   if ((NULL == interface) || (NULL == member))
!     goto cleanup;
  
    XD_DEBUG_MESSAGE ("Event received: %d %s %s %s %s %s",
                    mtype, uname, path, interface, member,
***************
*** 1210,1220 ****
                             args);
  
          /* Add uname, path, interface and member to the event.  */
!         event.arg = Fcons ((member == NULL ? Qnil : build_string (member)),
!                            event.arg);
!         event.arg = Fcons ((interface == NULL
!                             ? Qnil : build_string (interface)),
!                            event.arg);
          event.arg = Fcons ((path == NULL ? Qnil : build_string (path)),
                             event.arg);
          event.arg = Fcons ((uname == NULL ? Qnil : build_string (uname)),
--- 1211,1218 ----
                             args);
  
          /* Add uname, path, interface and member to the event.  */
!         event.arg = Fcons (build_string (member), event.arg);
!         event.arg = Fcons (build_string (interface), event.arg);
          event.arg = Fcons ((path == NULL ? Qnil : build_string (path)),
                             event.arg);
          event.arg = Fcons ((uname == NULL ? Qnil : build_string (uname)),
***************
*** 1235,1241 ****
       value = CDR_SAFE (value);
      }
  
!   /* Cleanup.  */
    dbus_message_unref (dmessage);
    RETURN_UNGCPRO (Qnil);
  }
--- 1233,1239 ----
       value = CDR_SAFE (value);
      }
  
!  cleanup:
    dbus_message_unref (dmessage);
    RETURN_UNGCPRO (Qnil);
  }