fix 12099 -- quick review?

[Rayson Ho]

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