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

Re: svn commit: r38193 - trunk/subversion/libsvn_subr

From: Stefan Sperling <stsp_at_elego.de>
Date: Thu, 25 Jun 2009 16:24:49 +0100

On Thu, Jun 25, 2009 at 04:11:01PM +0100, Julian Foad wrote:
> Stefan Sperling wrote:
> > On Thu, Jun 25, 2009 at 03:05:01PM +0100, Julian Foad wrote:
> > > Would there be an advantage in taking a length parameter instead of an
> > > end pointer? I think you would still need the equivalent assertion
> > > (length >= 0), because C doesn't protect against passing a negative
> > > number even if the data type is "unsigned".
> >
> > But that's a good reason for consistenly using unsinged.
> > On all levels. If the function already wants unsinged, the caller
> > should try to pass unsigned, too, without casting, anywhere.
>
> Unfortunately that doesn't work as well as you'd hope. See, for example,
> this discussion:
> <http://blogs.msdn.com/matt_pietrek/archive/2004/11/12/256775.aspx>...
> starting with the comment from "Adrian" half way down.

He complains about having to use casts to eliminate compiler
warnings. Well, that's probably a problem with the compiler.

He also says you cannot reliably subtract two unsinged numbers
from one another. But the same holds for signed values. Both can
wrap around.

> > > > /* Return the first eol marker found in [@a buf, @a endp) as a
> > > > * NUL-terminated string, or NULL if no eol marker is found.
> > > > *
> > > > * If the last valid character of @a buf is the first byte of a
> > > > * potentially two-byte eol sequence, just return "\r", that is,
> > >
> > > Shouldn't it return what's there (which could be "\n" or "\r") rather
> > > than always "\r" ...
> > >
> > > > * assume @a buf represents a CR-only file. This is correct for callers
> > >
> > > ... and "CR-only or LF-only"?
> >
> > I don't know. The function has always been doing this, even
> > while it was still a static function in libsvn_diff.
> > I don't want to change this behaviour because I'd have to tweak
> > the callers, too.
>
> I looked at the implementation. It does what I thought it should. The
> doc string's wrong.

Now I've looked at it, too. :)
I agree that the docstring is fairly unclear and does not
match the implementation. Do you want to adjust the docstring?

Stefan
Received on 2009-06-25 17:25:15 CEST

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.