[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] extend svn_subst_translate_string() to record whether re-encoding and/or line ending translation were performed (v. 5.2)

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Mon, 20 Dec 2010 10:11:15 +0000

On Sun, 2010-12-19 at 16:22 -0800, Danny Trebbien wrote:
> Branko Čibej <brane <at> xbc.nu> writes:
> > > Hi Daniel.
> > >
> > > I haven't looked at your patch v5 yet, because this Git diff doesn't
> > > look quite right. How will this memcmp expression work properly if the
> > > two strings being compared are different lengths?
> > >> - if (translated_eol != NULL && DIFFERENT_EOL_STRINGS(eol_str, eol_str_len,
> > >> - newline_buf, newline_len))
> > >> + if (translated_eol != NULL &&
> > >> + memcmp(eol_str,
> > >> + newline_buf,
> > >> + eol_str_len < newline_len ? eol_str_len : newline_len) != 0)
> >
> > Not very well, because it's prefix matching that doesn't actually take
> > different string lengths into account and you can get false matches.
> > e.g., if eol_str == '\r' (ancient mac mode) and newline_bug == '\r\n'
> > ... what you want is more like:
> >
> > if (translated_eol != NULL &&
> > (eol_str_len != newline_len ||
> > memcmp(eol_str, newline_buff, eol_str_len) != 0))
> >
>
> Ack! I didn't see your message until now. Yes, you are right.

Hi Danny. Please could you enhance your tests so that they can detect
this error (so that they will fail if this error is present).

Thanks.
- Julian

> Attached is version 5.2 of the patch.
Received on 2010-12-20 11:11:55 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.