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". If you want to use a length
for other reasons, such as consistency with other APIs, that's fine of
course.
Also:
> /* 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"?
> * that pass an entire file at once, and is no more likely to be
> * incorrect than correct for any caller that doesn't.
> *
> * @since New in 1.7
> */
> const char *
> svn_subst_detect_eol(char *buf, char *endp);
- Julian
> > On Thu, Jun 25, 2009 at 13:52, Stefan Sperling <stsp_at_elego.de> wrote:
> > >
> > > Author: stsp
> > > Date: Thu Jun 25 04:52:42 2009
> > > New Revision: 38193
> > >
> > > Log:
> > > Add an assertion to guard against invalid use of svn_subst_detect_eol().
> > >
> > > * subversion/libsvn_subr/subst.c
> > > (svn_subst_detect_eol): We should consider converting this function to
> > > take a pointer and a buffer size argument, instead of two pointers which
> > > define the buffer's boundaries. For now, just add an assert which if
> > > triggered indicates a bug in the caller.
> > >
> > > Modified:
> > > trunk/subversion/libsvn_subr/subst.c
> > >
> > > Modified: trunk/subversion/libsvn_subr/subst.c
> > > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/subst.c?pathrev=38193&r1=38192&r2=38193
> > > ==============================================================================
> > > --- trunk/subversion/libsvn_subr/subst.c Wed Jun 24 14:43:19 2009 (r38192)
> > > +++ trunk/subversion/libsvn_subr/subst.c Thu Jun 25 04:52:42 2009 (r38193)
> > > @@ -1657,7 +1657,10 @@ svn_subst_find_eol_start(char *buf, apr_
> > > const char *
> > > svn_subst_detect_eol(char *buf, char *endp)
> > > {
> > > - const char *eol = svn_subst_find_eol_start(buf, endp - buf);
> > > + const char *eol;
> > > +
> > > + SVN_ERR_ASSERT_NO_RETURN(buf <= endp);
> > > + eol = svn_subst_find_eol_start(buf, endp - buf);
> > > if (eol)
> > > {
> > > if (*eol == '\n')
> > >
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2365293
Received on 2009-06-25 16:05:25 CEST