Re: fix 12099 -- quick review?

[Rayson Ho <raysonlogin at gmail dot com>]

Please let me know if there are other issues, and if not this is the
final version that will be pushed back to the main tree.

Thanks David & Frank for the review comments.

Rayson


diff --git a/tapsets.cxx b/tapsets.cxx
index a8970bc..91e5eba 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -30,6 +30,7 @@
 #include <cstdlib>
 #include <algorithm>
 #include <deque>
+#include <fstream>
 #include <iostream>
 #include <map>
 #include <set>
@@ -56,6 +57,7 @@ extern "C" {
 #include <math.h>
 #include <regex.h>
 #include <unistd.h>
+#include <limits.h>

 #define __STDC_FORMAT_MACROS
 #include <inttypes.h>
@@ -5823,6 +5825,72 @@ dwarf_builder::build(systemtap_session & sess,
         }

       user_path = find_executable (module_name); // canonicalize it
+
+
+      // if the executable starts with "#!", we look for the
interpreter of the script
+      {
+         ifstream script_file (user_path.c_str () );
+
+         if (script_file.good ())
+         {
+           string line;
+
+           getline (script_file, line);
+
+           if (line.compare (0, sizeof("#!")-1, "#!") == 0)
+           {
+              string path_head = line.substr(sizeof("#!")-1);
+
+              // remove white spaces at the beginning of the string
+              size_t p2 = path_head.find_first_not_of(" \t");
+
+              if (p2 != string::npos)
+              {
+                string path = path_head.substr(p2);
+
+                // remove white spaces at the end of the string
+                p2 = path.find_last_not_of(" \t\n");
+                if (string::npos != p2)
+                  path.erase(p2+1);
+
+                user_path = find_executable (path);
+
+                struct stat st;
+
+                if (access (user_path.c_str(), X_OK) == 0
+                  && stat (user_path.c_str(), &st) == 0
+                  && S_ISREG (st.st_mode)) // see find_executable()
+                {
+                  if (sess.verbose > 1)
+                    clog << "Expanded process(\"" << module_name << "\") to "
+                         << "process(\"" << user_path << "\")" << endl;
+
+                  assert (location->components.size() > 0);
+                  assert (location->components[0]->functor == TOK_PROCESS);
+                  assert (location->components[0]->arg);
+                  literal_string* lit =
dynamic_cast<literal_string*>(location->components[0]->arg);
+                  assert (lit);
+
+                  // synthesize a new probe_point, with the expanded string
+                  probe_point *pp = new probe_point (*location);
+                  probe_point::component* ppc = new
probe_point::component (TOK_PROCESS,
+
      new literal_string (user_path.c_str()));
+                  ppc->tok = location->components[0]->tok; //
overwrite [0] slot, pattern matched above
+                  pp->components[0] = ppc;
+
+                  probe* new_probe = new probe (*base, pp);
+
+                  derive_probes (sess, new_probe, finished_results);
+
+                  script_file.close();
+                  return;
+                }
+              }
+           }
+         }
+         script_file.close();
+      }
+
       if (get_param (parameters, TOK_LIBRARY, library_name))
 	{
 	  module_name = find_executable (library_name, "LD_LIBRARY_PATH");

diff --git a/testsuite/systemtap.base/tracescripts.exp
b/testsuite/systemtap.base/tracescripts.exp
index e69de29..b7bd8f6 100644
--- a/testsuite/systemtap.base/tracescripts.exp
+++ b/testsuite/systemtap.base/tracescripts.exp
@@ -0,0 +1,11 @@
+set stap_path $env(SYSTEMTAP_PATH)/stap
+
+exec echo "#!$stap_path" > tracescripts.sh
+
+if {[catch {exec ../../stap -L
{process("tracescripts.sh").function("main")}} res]} {
+        fail "tracescripts $res"
+} else {
+        pass "tracescripts"
+}
+
+catch {exec rm -f tracescripts.sh}



On Mon, Dec 6, 2010 at 2:16 PM, Rayson Ho <raysonlogin@gmail.com> wrote:
> Thanks David for the comment. I originally copied some of the I/O code
> from the same file & other parts of systemtap.
>
> I will change the code to use C++ strings, and when the testcase is
> ready I will check it in.
>
> Rayson
>
>
>
> On Mon, Dec 6, 2010 at 1:09 PM, David Smith <dsmith@redhat.com> wrote:
>> On 12/06/2010 09:27 AM, Rayson Ho wrote:
>>> Thanks Frank for the review, here is the C++ iostreams version.
>>>
>>> Rayson
>>
>> I'm not sure you went far enough. ?You use C++ iostreams, but not C++
>> strings.
>>
>>> + ? ? ? ? if (script_file.good())
>>> + ? ? ? ? {
>>> + ? ? ? ? ? char buffer[PATH_MAX + 1 + 2]; // max path len + \n + #!
>>> +
>>> + ? ? ? ? ? script_file.getline(buffer, sizeof(buffer));
>>
>> Which might not be enough storage if we've got "#! ? {PATH}". ?So, why
>> not use the C++ string version of 'getline', like this:
>>
>> ? ? ? ? ? ? ?string buffer;
>> ? ? ? ? ? ? ?getline(script_file, buffer);
>>
>>> + ? ? ? ? ? if (buffer[0] != '\0')
>>> + ? ? ? ? ? {
>>> + ? ? ? ? ? ? size_t plen = strlen (buffer);
>>> +
>>> + ? ? ? ? ? ? if (plen > sizeof ("#!") && memcmp (buffer, "#!", sizeof
>>> ("#!")-1) == 0)
>>
>> I tend to treat strings as strings, so I would have written the above as:
>> ? ? ? ? ? ? ? ?if (strcmp(buffer, "#!") == 0)
>>
>> (or if you were using a C++ string you would use the .compare() function)
>>
>>> + ? ? ? ? ? ? {
>>> + ? ? ? ? ? ? ? ?// remove white spaces at the end of the string
>>> + ? ? ? ? ? ? ? ?plen--;
>>> +
>>> + ? ? ? ? ? ? ? ?while (buffer[plen] == '\n' || buffer[plen] == ' ' ||
>>> buffer[plen] == '\t')
>>> + ? ? ? ? ? ? ? ?{
>>> + ? ? ? ? ? ? ? ? ?buffer[plen] = '\0';
>>> + ? ? ? ? ? ? ? ? ?plen--;
>>> + ? ? ? ? ? ? ? ?}
>>
>> If you were using a C++ string, you could replace the above with
>>
>> ? ? ? ? ? ? ? ? ? // remove white spaces at the end of the string
>> ? ? ? ? ? ? ? ? ? size_t p2 = buffer.find_last_not_of(" \t\n");
>> ? ? ? ? ? ? ? ? ? if (string::npos != p2)
>> ? ? ? ? ? ? ? ? ? ? ? s.erase(p2+1);
>>
>> and similar changes through the rest of it.
>>
>> --
>> David Smith
>> dsmith@redhat.com
>> Red Hat
>> http://www.redhat.com
>> 256.217.0141 (direct)
>> 256.837.0057 (fax)
>>
>