RHBZ1269062 redux, less quote elevator_name

dsmith pointed out that switching to a quoted string is not always the
right thing to do, esp. from a backward compatibility point of view.
We now formalize a policy guideline in DEVGUIDE that *_string2 be used
when pointers should be valid but might be 0, which is the situation
with these probe points.

diff --git a/NEWS b/NEWS
index 82ed80748e01e7b94be0e871a85293a3bdc6aafd..72620126469d6be423854868038edb27a74de57e 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,8 +1,5 @@
 * What's new in version 2.9, 2015-09-30
 
-- The ioscheduler.* tapset functions return a quoted elevator_name string
-  rather than unquoted, since the underlying kernel pointer could be null.
-
 - SystemTap now uses symbols from /proc/kallsyms when kernel debuginfo is not
   available.
 

diff --git a/tapset/DEVGUIDE b/tapset/DEVGUIDE
index 2e3a6bd4cd7e411ba704ad984f6ccb3fe9c3fb9a..570e1ff84ac863fe4ccd4be66870688990e55992 100644 (file)
--- a/tapset/DEVGUIDE
+++ b/tapset/DEVGUIDE
@@ -81,6 +81,14 @@ SystemTap language, like access structure fields in some contexts, follow
 linked lists, etc. You can use auxiliary functions defined in other tapsets
 or write your own.
 
+Take care when target variables may have invalid, unavailable, or
+esoteric values, or are passed via pointers.  The a $pointer->field
+dereference can fail, so consider wrapping a try { } catch { } around
+it.  For a char* being converted to a string, use {kernel,user}_string
+only if the pointer is guaranteed valid.  Use {kernel,user}_string2($ptr,"")
+if the pointer might be NULL but should otherwise be good.  Use
+{kernel,user}_string_quoted($ptr) to handle any possible pointer.
+
 In the following example, copy_process() returns a pointer to the
 task_struct for the new process.  Note that the process ID of the new
 process is retrieved by calling task_pid() and passing it the task_struct
@@ -278,6 +286,12 @@ into the body of the embedded-C function, to mark it as /* unprivileged */
 (see above) and to have the systemtap translator automatically add a call to
 assert_is_myproc() to the beginning of the function.
 
+Add the string
+    /* unmodified-fnargs */
+into the body of the embedded-C function to inform the translator
+that any string inputs and return values may be passed by reference
+rather than copy.  This enables optimization.
+
 Add the string
     /* unmangled */
 into a legacy (pre-SystemTap 1.8) embedded-C function that needs to

diff --git a/tapset/linux/ioscheduler.stp b/tapset/linux/ioscheduler.stp
index 07562ff1f2b462c4ba78c7d140f925245a4d11dc..8da4a4c0fd7f17c73b30c603c4d5f52f8da583b0 100644 (file)
--- a/tapset/linux/ioscheduler.stp
+++ b/tapset/linux/ioscheduler.stp
@@ -23,10 +23,10 @@ probe ioscheduler.elv_next_request = kernel.function("blk_peek_request") !,
                                     kernel.function("elv_next_request")
 {
        name = "elv_next_request"
-       elevator_name = kernel_string_quoted(
+       elevator_name = kernel_string2(
                @choose_defined($q->elevator->type->elevator_name,
                        @choose_defined($q->elevator->elevator_type->elevator_name,
-                               $q->elevator->elevator_name)))
+                                        $q->elevator->elevator_name)), "")
 }
 
 /**
@@ -73,10 +73,10 @@ probe ioscheduler.elv_completed_request
        = kernel.function("elv_completed_request")
 {
        name = "elv_completed_request"
-       elevator_name = kernel_string_quoted(
+       elevator_name = kernel_string2(
                @choose_defined($q->elevator->type->elevator_name,
                        @choose_defined($q->elevator->elevator_type->elevator_name,
-                               $q->elevator->elevator_name)))
+                                        $q->elevator->elevator_name)), "")
        if($rq == 0)  {
                disk_major = -1
                disk_minor = -1
@@ -111,10 +111,10 @@ probe ioscheduler.elv_add_request.kp =
                kernel.function("__elv_add_request")
 {
        name = "elv_add_request"
-       elevator_name = kernel_string_quoted(
+       elevator_name = kernel_string2(
                @choose_defined($q->elevator->type->elevator_name,
                        @choose_defined($q->elevator->elevator_type->elevator_name,
-                               $q->elevator->elevator_name)))
+                                        $q->elevator->elevator_name)), "")
        q = $q
        if($rq == 0)  {
                disk_major = -1
@@ -149,10 +149,10 @@ probe ioscheduler.elv_add_request.tp = kernel.trace("block_rq_insert") ?
 {
        name = "elv_add_request"
        q =  $q
-       elevator_name = kernel_string_quoted(
+       elevator_name = kernel_string2(
                @choose_defined($q->elevator->type->elevator_name,
                        @choose_defined($q->elevator->elevator_type->elevator_name,
-                               $q->elevator->elevator_name)))
+                                        $q->elevator->elevator_name)), "")
        rq = $rq
 
        if ($rq == 0 || $rq->rq_disk ==0) {
@@ -197,10 +197,10 @@ probe ioscheduler_trace.elv_completed_request
                        = kernel.trace("block_rq_complete") ?
 {
        name = "elv_completed_request"
-       elevator_name = kernel_string_quoted(
+       elevator_name = kernel_string2(
                @choose_defined($q->elevator->type->elevator_name,
                        @choose_defined($q->elevator->elevator_type->elevator_name,
-                               $q->elevator->elevator_name)))
+                                        $q->elevator->elevator_name)), "")
        rq = $rq
 
        if ($rq == 0 || $rq->rq_disk ==0) {
@@ -231,10 +231,10 @@ probe ioscheduler_trace.elv_issue_request
                        = kernel.trace("block_rq_issue") ?
 {
        name = "elv_issue_request"
-       elevator_name = kernel_string_quoted(
+       elevator_name = kernel_string2(
                @choose_defined($q->elevator->type->elevator_name,
                        @choose_defined($q->elevator->elevator_type->elevator_name,
-                               $q->elevator->elevator_name)))
+                                        $q->elevator->elevator_name)), "")
        rq = $rq
 
        if ($rq == 0 || $rq->rq_disk ==0) {
@@ -265,10 +265,10 @@ probe ioscheduler_trace.elv_requeue_request
                        = kernel.trace("block_rq_requeue") ?
 {
        name = "elv_requeue_request"
-       elevator_name = kernel_string_quoted(
+       elevator_name = kernel_string2(
                @choose_defined($q->elevator->type->elevator_name,
                        @choose_defined($q->elevator->elevator_type->elevator_name,
-                               $q->elevator->elevator_name)))
+                                        $q->elevator->elevator_name)), "")
        rq = $rq
 
        if ($rq == 0 || $rq->rq_disk ==0) {
@@ -298,10 +298,10 @@ probe ioscheduler_trace.elv_abort_request
                        = kernel.trace("block_rq_abort") ?
 {
        name = "elv_abort_request"
-       elevator_name = kernel_string_quoted(
+       elevator_name = kernel_string2(
                @choose_defined($q->elevator->type->elevator_name,
                        @choose_defined($q->elevator->elevator_type->elevator_name,
-                               $q->elevator->elevator_name)))
+                                        $q->elevator->elevator_name)), "")
        rq = $rq
 
        if ($rq == 0 || $rq->rq_disk ==0) {