Mail Archives: cygwin/2012/03/28/13:34:49
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
- Raw text -