RE: [PATCH] Issue 3942 - Provide new subcommand on svnadmin to create a lock

Noorul Islam K M wrote:
> + SVN_ERR(svn_stringbuf_from_file2(&file_contents, comment_file_name, pool));
> + comment = file_contents->data;
> +
> + /* Enforce that the comment be xml-escapable. */
> + if (comment)

There doesn't seem to be any way that 'comment' could be null in your
current patch, since svn_stringbuf_from_file2() promises to return "a
string". (But maybe you will make the comment optional in a subsequent
patch.)

> + {
> + if (! svn_xml_is_xml_safe(comment, strlen(comment)))

Instead of 'comment' and 'strlen(comment)' you could use
file_contents->data and file_contents->len. That would also catch the
case where there's a null character in the file.

> + return svn_error_create
> + (SVN_ERR_XML_UNESCAPABLE_DATA, NULL,
> + _("Lock comment contains illegal characters"));
> + }
> +
> + SVN_ERR(svn_fs_youngest_rev(&revnum, fs, subpool));

Don't get the youngest revnum ...

> + SVN_ERR(svn_utf_cstring_to_utf8(&lock_path_utf8, lock_path, subpool));
> +
> + SVN_ERR(svn_repos_fs_lock(&lock, repos, lock_path_utf8,
> + NULL, /* token */
> + comment,
> + 0, /* is_dav_comment */
> + 0, /* No expiration time. */
> + revnum,

... because that will only make this call fail if there's a commit
happening at the same time, where otherwise this would succeed. Just
pass SVN_INVALID_REVNUM.

> + FALSE, subpool));

- Julian
Received on 2011-07-15 16:33:28 CEST