Either email or by going to the link below and commenting using the review tool. Frankly, I prefer email except if you need to use the ability to comment on particular lines in context. Though I can see how others might prefer that ALL review correspondence go through the tool, to keep the conversations together.

As for robustness, why wouldn't strncmp be perfectly robust as I've used it? What do you imagine would go wrong if source was shorter than pattern, other than wasting a few cycles in strncmp?

-- lg


On Jan 25, 2010, at 3:51 PM, Brian Budge wrote:

Hi Larry -

Just to be robust, would it make sense to make sure that source is at
least as long as pattern? Also, is this the correct way to respond to
a code review (in email)?

Brian

On Mon, Jan 25, 2010 at 3:34 PM, <larry...@...> wrote:

Reviewers: ,

Description:
Many times I've been plagued by inexplicable crashes in loadshader.cpp,
right at the call to boost::starts_with. I've wasted hours and hours
and been unable to figure out why it boost::startswith(x,y) should fail
when x and y are valid std::strings. So I finally wrote my own
(trivially) just to see if that happens, and I no longer crash. (Making
me able to move on to the real crash I'm supposed to be debugging. :-)


Please review this at http://codereview.appspot.com/194067/show

Affected files:
src/liboslexec/loadshader.cpp


Index: src/liboslexec/loadshader.cpp
===================================================================
--- src/liboslexec/loadshader.cpp (revision 544)
+++ src/liboslexec/loadshader.cpp (working copy)
@@ -232,13 +232,21 @@



+inline bool
+starts_with (const std::string &source, const std::string &pattern)
+{
+ return ! strncmp (source.c_str(), pattern.c_str(), pattern.length());
+}
+
+
+
// If the string 'source' begins with 'pattern', erase the pattern from
// the start of source and return true. Otherwise, do not alter source
// and return false.
inline bool
extract_prefix (std::string &source, const std::string &pattern)
{
- if (boost::starts_with (source, pattern)) {
+ if (starts_with (source, pattern)) {
source.erase (0, pattern.length());
return true;
}


--
You received this message because you are subscribed to the Google Groups
"OSL Developers" group.
To post to this group, send email to osl...@....
To unsubscribe from this group, send email to
osl...@....
For more options, visit this group at
http://groups.google.com/group/osl-dev?hl=en.

--
Larry Gritz
l...@...