On Thu, Jun 25, 2009 at 03:05:01PM +0100, Julian Foad wrote:
> Stefan Sperling wrote:
> > On Thu, Jun 25, 2009 at 02:55:15PM +0200, Greg Stein wrote:
> > > Shouldn't those arguments be 'const' ?
> > Yeah they should be.
> > Don't blame me, I didn't write that code, I just moved it :P
> > I'll consider changing the function's parameter anyway as
> > outlined in the log message (to const char *buf and apr_size_t len).
> 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.
> If you want to use a length
> for other reasons, such as consistency with other APIs, that's fine of
It's just consistency, really. And also because an API that allows you
to simply pass sizeof(buf) instead of pointer offsets is much easier
to use, IMO. No brain-power involved for the programmer, because
the programmer can assume that the function will take care not to
overrun the buffer. That's just a pattern I've grown used to.
> > /* 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.
Received on 2009-06-25 16:30:20 CEST