X-Recipient: archive-cygwin AT delorie DOT com X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Date: Wed, 28 Mar 2012 10:34:49 -0700 From: Gary Johnson To: cygwin AT cygwin DOT com Subject: Re: rcs 5.8-1 checks out wrong version of file when using similar mark symbols Message-ID: <20120328173449.GA28489@phoenix> Mail-Followup-To: cygwin AT cygwin DOT com References:

<4F717637 DOT 80703 AT lysator DOT liu DOT se> <4F71791C DOT 8010707 AT lysator DOT liu DOT se> <20120328065524 DOT GA27483 AT phoenix> <4F72D956 DOT 50009 AT lysator DOT liu DOT se> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <4F72D956.50009@lysator.liu.se> User-Agent: Mutt/1.5.20 (2009-06-14) X-IsSubscribed: yes Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: cygwin-owner AT cygwin DOT com Mail-Followup-To: cygwin AT cygwin DOT com Delivered-To: mailing list cygwin AT cygwin DOT com 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