This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: fix 12099 -- quick review?
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)