Re: svn commit: rev 1888 - trunk/subversion/svnadmin trunk/subversion/include trunk/subversion/libsvn_subr trunk/subversion/libsvn_repos

On Mon, May 06, 2002 at 05:08:48PM -0500, sussman@tigris.org wrote:
>...
> +++ trunk/subversion/libsvn_repos/dump.c Mon May 6 17:08:47 2002
>...
> struct edit_baton
> {
> - svn_stringbuf_t *path; /* this will almost always be "/" */
> - apr_file_t *file;
> - svn_revnum_t rev;
> - svn_fs_t *fs;
> + /* The path which implicitly prepends all full paths coming into
> + this editor. This will almost always be "" or "/". */
> + svn_stringbuf_t *path;

This stringbuf nonsense should go away. The path should be a 'const char *'
and you should be using svn_path_join() rather than the add_component crap.

>...
> @@ -276,38 +280,54 @@
> /* ### someday write a node-content-checksum here. */
>
> /* This is the last header before we dump the content. */
> - apr_file_printf (file,
> - SVN_REPOS_DUMPFILE_NODE_CONTENT_LENGTH ": %ld\n\n",
> - (long int) content_length);
> + svn_stream_printf (stream, pool,
> + SVN_REPOS_DUMPFILE_NODE_CONTENT_LENGTH ": %ld\n\n",
> + (long int) content_length);

Woah. Toss that %ld and the cast. Use the appropriate symbolic constant for
the format.

> /* Dump property content. */
> - apr_file_printf (file, propstring->data);
> + svn_stream_printf (stream, pool, propstring->data);

Badness. You want a simple svn_stream_write() here. You really would not
want to interpret the property data as a printf() format code(!)

>...
> - apr_file_printf (file, "\n\n"); /* ### needed? */
> + svn_stream_printf (stream, pool, "\n\n"); /* ### needed? */

printf is a bit heavy here. Should be a simple write().

>...
> @@ -553,14 +572,14 @@
>
> /* ### someday write a revision-content-checksum */
>
> - apr_file_printf (file,
> + svn_stream_printf (stream, pool,
> SVN_REPOS_DUMPFILE_REVISION_NUMBER ": %ld\n", rev);

Use the right symbolic constant for the format! (SVN_REVNUM_T_FMT)

> - apr_file_printf (file,
> + svn_stream_printf (stream, pool,
> SVN_REPOS_DUMPFILE_REVISION_CONTENT_LENGTH ": %ld\n\n",
> (long int) encoded_prophash->len);

Eek. Similar problem here.

Karl just got done fixing all this, and you put 'em back in :-)

> - apr_file_printf (file,
> + svn_stream_printf (stream, pool,
> encoded_prophash->data);

Should be a write()

> - apr_file_printf (file, "\n");
> + svn_stream_printf (stream, pool, "\n");

Probably a write() again.

>...
> @@ -601,8 +620,9 @@
>
> /* Write out "general" metadata for the dumpfile. In this case, a
> magic string followed by a dumpfile format version. */
> - apr_file_printf (file, SVN_REPOS_DUMPFILE_MAGIC_HEADER ": ");
> - apr_file_printf (file, "%d\n\n", SVN_REPOS_DUMPFILE_FORMAT_VERSION);
> + svn_stream_printf (stream, pool, SVN_REPOS_DUMPFILE_MAGIC_HEADER ": ");
> + svn_stream_printf (stream, pool, "%d\n\n",
> + SVN_REPOS_DUMPFILE_FORMAT_VERSION);

This can be a single print:

  svn_stream_printf(stream, pool,
                    SVN_REPOS_DUMPFILE_MAGIC_HEADER ": %d\n\n",
                    SVN_REPOS_DUMPFILE_FORMAT_VERSION);

or even better:

#define HDR SVN_REPOS_DUMPFILE_MAGIC_NUMBER ": " \
            APR_STRINGIFY(SVN_REPOS_DUMPFILE_FORMAT_VERSION) "\n\n"
#define HDRLEN (sizeof(HDR)-1)
  svn_stream_write(..., HDR, HDRLEN)

(or somesuch)

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