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

Re: svn commit: rev 3790 - in trunk/subversion: include libsvn_subr tests/libsvn_subr

From: Greg Stein <gstein_at_lyra.org>
Date: 2002-11-15 01:00:09 CET

Heh. My previous comments don't really apply if you're reading from a
stringbuf, since you properly don't want to increment buf->data.

Personally, I would have created two stream types. One for reading, and one
for writing, rather than mixing them like this. With this mixed version, you
*have* to have a stringbuf. And a stringbuf's data is non-const, so you're
going to have a hard time reading from a constant string. Not to mention
that a stringbuf structure strictly needs a pool value, etc etc.

On Thu, Nov 14, 2002 at 03:30:20PM -0600, cmpilato@tigris.org wrote:
>...
> +++ trunk/subversion/include/svn_io.h Thu Nov 14 15:29:58 2002
>...
> +/* Return a generic stream connected to STR. Allocate the stream in
> + POOL. When used as readable stream, STR should contain the data
> + you wish to read. When used a writable stream, STR should be
> + non-NULL, initialized by the stringbuf creation function of your
> + choice, and writes to this stream will append data to STR. */

STR should *always* be non-NULL. It isn't an optinal parameter, so that
phrasing is a bit weird.

>...
> +++ trunk/subversion/libsvn_subr/io.c Thu Nov 14 15:30:02 2002
>...
> + if (! btn->str)
> + {
> + *len = 0;
> + return SVN_NO_ERROR;
> + }
>...
> + if (! btn->str)
> + {
> + *len = 0;
> + return svn_error_create (SVN_ERR_INCORRECT_PARAMS, 0, NULL,
> + "No receiving string available");
> + }

Both of these are rather non-sensical. btn->str should always exist. As with
any of our code, just rip this out. If some idiot passes a NULL str, then
they'll get a segfault right quick, so the failure mode is quite fine.

>...
> +svn_stream_from_stringbuf (svn_stringbuf_t *str,
> + apr_pool_t *pool)
> {
> svn_stream_t *stream;
> struct string_stream_baton *baton;
>
> - if (! (data && len))
> + if (! str)
> return svn_stream_empty (pool);

Eh? Why bother. If the user has a NULL pointer, they should call
svn_stream_empty() themselves. Keep the interface as requiring a pointer.
(not to mention this behavior wasn't doc'd in the header)

>...

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Nov 15 01:47:07 2002

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.