all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* dbusbind.c SIGSEGV fix and minor cleanup
@ 2008-03-18  9:47 David Hansen
  2008-03-21  2:00 ` David Hansen
  2008-03-21 16:27 ` Michael Albinus
  0 siblings, 2 replies; 5+ messages in thread
From: David Hansen @ 2008-03-18  9:47 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 218 bytes --]

Hello,

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'.

David


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 2794 bytes --]

*** dbusbind.c.~1.22.~	2008-02-17 01:40:28.000000000 +0100
--- dbusbind.c	2008-03-18 10:35:30.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))
!     RETURN_UNGCPRO (Qnil);
  
    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)),

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: dbusbind.c SIGSEGV fix and minor cleanup
  2008-03-18  9:47 dbusbind.c SIGSEGV fix and minor cleanup David Hansen
@ 2008-03-21  2:00 ` David Hansen
  2008-03-21 16:27 ` Michael Albinus
  1 sibling, 0 replies; 5+ messages in thread
From: David Hansen @ 2008-03-21  2:00 UTC (permalink / raw)
  To: emacs-devel

On Tue, 18 Mar 2008 10:47:11 +0100 David Hansen wrote:

> emacs crashes (in `strcpy') when receiving dbus messages with some empty
> content.

Shall I write a more detailed description of the patch?  I mean,
something should be done, a SIGSEGV is definitively a bug.

David





^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: dbusbind.c SIGSEGV fix and minor cleanup
  2008-03-18  9:47 dbusbind.c SIGSEGV fix and minor cleanup David Hansen
  2008-03-21  2:00 ` David Hansen
@ 2008-03-21 16:27 ` Michael Albinus
  2008-03-22 12:55   ` David Hansen
  1 sibling, 1 reply; 5+ messages in thread
From: Michael Albinus @ 2008-03-21 16:27 UTC (permalink / raw)
  To: emacs-devel

David Hansen <david.hansen@gmx.net> 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'.

The patch looks OK to me.

One could argue, that in case interface or member is NULL, this could be
regarded as wildcards, and all registered functions which match
otherwise shall be applied. But I agree with you, that only messages,
which are sent to a specified interface and its member, shall be taken.

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 already.

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

> David

Thanks, and best regards, Michael.




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: dbusbind.c SIGSEGV fix and minor cleanup
  2008-03-21 16:27 ` Michael Albinus
@ 2008-03-22 12:55   ` David Hansen
  2008-03-23 16:55     ` Michael Albinus
  0 siblings, 1 reply; 5+ messages in thread
From: David Hansen @ 2008-03-22 12:55 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1347 bytes --]

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

> David Hansen <david.hansen@gmx.net> 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  <david.hansen@gmx.net>

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 3091 bytes --]

*** 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);
  }

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: dbusbind.c SIGSEGV fix and minor cleanup
  2008-03-22 12:55   ` David Hansen
@ 2008-03-23 16:55     ` Michael Albinus
  0 siblings, 0 replies; 5+ messages in thread
From: Michael Albinus @ 2008-03-23 16:55 UTC (permalink / raw)
  To: emacs-devel

David Hansen <david.hansen@gmx.net> writes:

> 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.

Could you, please, show the message it did receive when crashing? You
could simplify the test by enabling traces in dbusbind.c. When you
compile Emacs with "make MYCPPFLAGS='-DDBUS_DEBUG'", all traces in
XD_DEBUG_MESSAGE are written to STDOUT.

Best regards, Michael.




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2008-03-23 16:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-18  9:47 dbusbind.c SIGSEGV fix and minor cleanup David Hansen
2008-03-21  2:00 ` David Hansen
2008-03-21 16:27 ` Michael Albinus
2008-03-22 12:55   ` David Hansen
2008-03-23 16:55     ` Michael Albinus

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.