Only call {set,clear}_semaphore probe function if they are not NULL

Message ID 1412976993-9465-1-git-send-email-sergiodj@redhat.com
State New, archived
Headers

Commit Message

Sergio Durigan Junior Oct. 10, 2014, 9:36 p.m. UTC
  This patch is a response to what I commented on:

  <https://sourceware.org/ml/gdb-patches/2014-10/msg00046.html>

When reviewing Jose's USDT probe support patches.  Basically, in his
patch he had to create dummy functions for the set_semaphore and the
clear_semaphore methods of probe_ops (gdb/probe.h), because those
functions were called inconditionally from inside gdb/breakpoint.c and
gdb/tracepoint.c.  However, the semaphore concept may not apply to all
types of probes, and this is the case here: USDT probes do not have
semaphores (although SDT probes do).

Anyway, this is a simple (almost obvious) patch to guard the call to
{set,clear}_semaphore.  It does not introduce any regression on a
Fedora 20 x86_64.

I will apply it in a few days in case there is no comment.

gdb/ChangeLog:
2014-10-10  Sergio Durigan Junior  <sergiodj@redhat.com>

	* breakpoint.c (bkpt_probe_insert_location): Call set_semaphore
	only if it is not NULL.
	(bkpt_probe_remove_location): Likewise, for clear_semaphore.
	* probe.h (struct probe_ops) <set_semaphore>: Update comment.
	(struct probe_ops) <clear_semaphore>: Likewise.
	* tracepoint.c (start_tracing): Call set_semaphore only if it is
	not NULL.
	(stop_tracing): Likewise, for clear_semaphore.
---
 gdb/breakpoint.c | 14 ++++++++------
 gdb/probe.h      |  4 ++--
 gdb/tracepoint.c |  6 ++++--
 3 files changed, 14 insertions(+), 10 deletions(-)
  

Comments

Sergio Durigan Junior Oct. 14, 2014, 6:48 p.m. UTC | #1
On Friday, October 10 2014, I wrote:

> This patch is a response to what I commented on:
>
>   <https://sourceware.org/ml/gdb-patches/2014-10/msg00046.html>
[...]
> I will apply it in a few days in case there is no comment.

Pushed.

  <https://sourceware.org/ml/gdb-cvs/2014-10/msg00043.html>
  

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3044916..6de5804 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -13696,9 +13696,10 @@  bkpt_probe_insert_location (struct bp_location *bl)
     {
       /* The insertion was successful, now let's set the probe's semaphore
 	 if needed.  */
-      bl->probe.probe->pops->set_semaphore (bl->probe.probe,
-					    bl->probe.objfile,
-					    bl->gdbarch);
+      if (bl->probe.probe->pops->set_semaphore != NULL)
+	bl->probe.probe->pops->set_semaphore (bl->probe.probe,
+					      bl->probe.objfile,
+					      bl->gdbarch);
     }
 
   return v;
@@ -13708,9 +13709,10 @@  static int
 bkpt_probe_remove_location (struct bp_location *bl)
 {
   /* Let's clear the semaphore before removing the location.  */
-  bl->probe.probe->pops->clear_semaphore (bl->probe.probe,
-					  bl->probe.objfile,
-					  bl->gdbarch);
+  if (bl->probe.probe->pops->clear_semaphore != NULL)
+    bl->probe.probe->pops->clear_semaphore (bl->probe.probe,
+					    bl->probe.objfile,
+					    bl->gdbarch);
 
   return bkpt_remove_location (bl);
 }
diff --git a/gdb/probe.h b/gdb/probe.h
index b4ff0a6..a128151 100644
--- a/gdb/probe.h
+++ b/gdb/probe.h
@@ -96,14 +96,14 @@  struct probe_ops
 
     /* Set the semaphore associated with the PROBE.  This function only makes
        sense if the probe has a concept of semaphore associated to a
-       probe.  */
+       probe, otherwise it can be set to NULL.  */
 
     void (*set_semaphore) (struct probe *probe, struct objfile *objfile,
 			   struct gdbarch *gdbarch);
 
     /* Clear the semaphore associated with the PROBE.  This function only
        makes sense if the probe has a concept of semaphore associated to
-       a probe.  */
+       a probe, otherwise it can be set to NULL.  */
 
     void (*clear_semaphore) (struct probe *probe, struct objfile *objfile,
 			     struct gdbarch *gdbarch);
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 51af2af..fc20063 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1858,7 +1858,8 @@  start_tracing (char *notes)
       t->number_on_target = b->number;
 
       for (loc = b->loc; loc; loc = loc->next)
-	if (loc->probe.probe != NULL)
+	if (loc->probe.probe != NULL
+	    && loc->probe.probe->pops->set_semaphore != NULL)
 	  loc->probe.probe->pops->set_semaphore (loc->probe.probe,
 						 loc->probe.objfile,
 						 loc->gdbarch);
@@ -1957,7 +1958,8 @@  stop_tracing (char *note)
 	     but we don't really care if this semaphore goes out of sync.
 	     That's why we are decrementing it here, but not taking care
 	     in other places.  */
-	  if (loc->probe.probe != NULL)
+	  if (loc->probe.probe != NULL
+	      && loc->probe.probe->pops->clear_semaphore != NULL)
 	    loc->probe.probe->pops->clear_semaphore (loc->probe.probe,
 						     loc->probe.objfile,
 						     loc->gdbarch);