On 2012-03-28, Peter Rosin wrote: > Gary Johnson skrev 2012-03-28 08:55: > > On 2012-03-27, Peter Rosin wrote: > >> But the point still stands, don't assume the original authors were > >> idiots, and dig into the reasons for them to not having used > >> strcmp from the start. > > > > I don't know, the "original" authors seem to have gotten it right, > > as version 5.7 works correctly on my Fedora system, and the function > > in question was added between versions 5.7 and 5.8. > > What are you trying to say here? That whoever it was that brought > rcs from 5.7 to 5.8 are a bunch of idiots? I'm sure not, but that's > what it sounds like...
I'm just saying that 5.7 works and 5.8 apparently does not. Draw you own conclusions. One doesn't have to be an idiot to make a mistake. The author of that function just may not have been thinking clearly when he or she wrote it, for any of a number of possible reasons. > Cheech, I just said that it looked suspect that strcmp was not used > from the start and that someone needed to look at the code and double- > check if strlen/strcmp was safe to use before running full-steam into > a segfault. > > So, go look at the code. I just did, and the suggested changes are > indeed broken since the id string is *not* guaranteed to be zero- > terminated. It appears that the original authors (of 5.8 of course, > that's the version we are discussing) are not idiots, since you can > neither use strlen on the id string nor can you use strcmp on it. > > However, it seems as if d->meaningful is zero-terminated (as far as > I can tell strcmp, via the STR_SAME macro, is used on it at other > locations in the code). I wasn't defending the proposed changes, just saying that just because code comes from a respected group of developers doesn't mean it can't contain errors. I didn't look at enough of the code to determine the characteristics of those variables, but I'll take your word for it. If d->meaningful is null-terminated, id-string is not, and id->size is the length of id->string, then this expression from the 5.8 source is wrong: 11: if (!strncmp (d->meaningful, id->string, id->size)) as it ignores any characters in d->meaningful after the first id->size characters. > So, this is probably safe for line 11: > > if ((strlen(d->meaningful) == id->size) && !strncmp(d->meaningful, > id->string, id->size)) > > If d->meaningful is not guaranteed to be zero-terminated, this bug > is not fixable from within rev_from_symbol(). Agreed. Regards, Gary -- Problem reports: http://cygwin.com/problems.html FAQ: http://cygwin.com/faq/ Documentation: http://cygwin.com/docs.html Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple
