You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kamesh Jayachandran <ka...@collab.net> on 2008/05/05 14:29:34 UTC
Re: [PATCH] svn_io_check_path, round 2
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi Daniel,
+1 to commit.
Thanks
With regards
Kamesh Jayachandran
Daniel Shahaf wrote:
> Daniel Shahaf wrote on Sat, 3 May 2008 at 00:19 +0300:
>> kameshj@tigris.org wrote on Fri, 2 May 2008 at 06:05 -0700:
>>> @@ -789,7 +789,8 @@ svn_client__make_local_parents(const cha
>>> apr_pool_t *pool)
>>> {
>>> svn_error_t *err;
>>> -
>>> + svn_node_kind_t orig_kind;
>>> + SVN_ERR(svn_io_check_path(path, &orig_kind, pool));
>> According to svn_io_check_path's docstring (both before and after the
>> recent change), when "intermediate directories on the way to @a path
>> don't exist", it returns an error
>
> I double checked; contrary to the docstring's promise, no error is
> returned in this case. I try to fix the docstring in the patch below by
> removing the distinction between "Path does not exist" and "Path's parent
> does not exist" and redefining the "error" and "svn_node_unknown" cases to
> match svn_node_kind_t's documentation.
>
> I'll commit in a few days, but posting here for sanity check.
>
> [[[
> Follow up to r30937.
>
> * subversion/include/svn_io.h
> (svn_io_check_path): Try again to match the docstring to the
> implementation.
> ]]]
>
> The complete patched docstring is:
>
> /** Determine the @a kind of @a path. @a path should be UTF-8 encoded.
> *
> * If @a path is a file, set @a *kind to @c svn_node_file.
> *
> * If @a path is a directory, set @a *kind to @c svn_node_dir.
> *
> * If @a path does not exist, set @a *kind to @c svn_node_none.
> *
> * If @a path exists but is none of the above, set @a *kind to @c
> * svn_node_unknown.
> *
> * If unable to determine @a path's kind, return an error, with @a *kind's
> * value undefined.
> *
> * Use @a pool for temporary allocations.
> *
> * @see svn_node_kind_t
> */
> svn_error_t *svn_io_check_path(const char *path,
> svn_node_kind_t *kind,
> apr_pool_t *pool);
>
> The diff is:
>
> Index: subversion/include/svn_io.h
> ===================================================================
> --- subversion/include/svn_io.h (revision 31020)
> +++ subversion/include/svn_io.h (working copy)
> @@ -79,15 +79,17 @@ typedef struct svn_io_dirent_t {
> *
> * If @a path is a directory, set @a *kind to @c svn_node_dir.
> *
> - * If @a path does not exist in its final component, set @a *kind to
> - * @c svn_node_none.
> + * If @a path does not exist, set @a *kind to @c svn_node_none.
> *
> - * If intermediate directories on the way to @a path don't exist, return
> - * an error, with @a *kind's value undefined.
> + * If @a path exists but is none of the above, set @a *kind to @c
> + * svn_node_unknown.
> *
> - * Otherwise, set @a *kind to @c svn_node_unknown.
> + * If unable to determine @a path's kind, return an error, with @a *kind's
> + * value undefined.
> *
> * Use @a pool for temporary allocations.
> + *
> + * @see svn_node_kind_t
> */
> svn_error_t *svn_io_check_path(const char *path,
> svn_node_kind_t *kind,
>
>
> Daniel
>
>> and leaves the value of orig_kind uninitialized.
>>
>> In svn_client__make_local_parents we cannot assume that the intermediate
>> directories exist; therefore, the above call might raise an error.
>> However, 'mkdir --parents' works in HEAD; an error is not raised.
>>
>>
>> This leaves me with two questions:
>>
>> * Should we be calling svn_io_check_path in
>> svn_client__make_local_parents ?
>>
>> * Is svn_io_check_path's docstring still wrong? (when it says errors
>> would be returned)
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFIHxnO3WHvyO0YTCwRAjO6AKCUaYo8Ot7myM8+Bx0o/330S7qpfQCdHvTW
fcJbF+aI1SLHqLu6c4M54aU=
=rpHf
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] svn_io_check_path, round 2
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Kamesh Jayachandran wrote on Mon, 5 May 2008 at 19:59 +0530:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi Daniel,
>
> +1 to commit.
>
r31031. Thanks for the sanity check,
Daniel
> Thanks
> With regards
> Kamesh Jayachandran
> Daniel Shahaf wrote:
> > Daniel Shahaf wrote on Sat, 3 May 2008 at 00:19 +0300:
> >> kameshj@tigris.org wrote on Fri, 2 May 2008 at 06:05 -0700:
> >>> @@ -789,7 +789,8 @@ svn_client__make_local_parents(const cha
> >>> apr_pool_t *pool)
> >>> {
> >>> svn_error_t *err;
> >>> -
> >>> + svn_node_kind_t orig_kind;
> >>> + SVN_ERR(svn_io_check_path(path, &orig_kind, pool));
> >> According to svn_io_check_path's docstring (both before and after the
> >> recent change), when "intermediate directories on the way to @a path
> >> don't exist", it returns an error
> >
> > I double checked; contrary to the docstring's promise, no error is
> > returned in this case. I try to fix the docstring in the patch below by
> > removing the distinction between "Path does not exist" and "Path's parent
> > does not exist" and redefining the "error" and "svn_node_unknown" cases to
> > match svn_node_kind_t's documentation.
> >
> > I'll commit in a few days, but posting here for sanity check.
> >
> > [[[
> > Follow up to r30937.
> >
> > * subversion/include/svn_io.h
> > (svn_io_check_path): Try again to match the docstring to the
> > implementation.
> > ]]]
> >
> > The complete patched docstring is:
> >
> > /** Determine the @a kind of @a path. @a path should be UTF-8 encoded.
> > *
> > * If @a path is a file, set @a *kind to @c svn_node_file.
> > *
> > * If @a path is a directory, set @a *kind to @c svn_node_dir.
> > *
> > * If @a path does not exist, set @a *kind to @c svn_node_none.
> > *
> > * If @a path exists but is none of the above, set @a *kind to @c
> > * svn_node_unknown.
> > *
> > * If unable to determine @a path's kind, return an error, with @a *kind's
> > * value undefined.
> > *
> > * Use @a pool for temporary allocations.
> > *
> > * @see svn_node_kind_t
> > */
> > svn_error_t *svn_io_check_path(const char *path,
> > svn_node_kind_t *kind,
> > apr_pool_t *pool);
> >
> > The diff is:
> >
> > Index: subversion/include/svn_io.h
> > ===================================================================
> > --- subversion/include/svn_io.h (revision 31020)
> > +++ subversion/include/svn_io.h (working copy)
> > @@ -79,15 +79,17 @@ typedef struct svn_io_dirent_t {
> > *
> > * If @a path is a directory, set @a *kind to @c svn_node_dir.
> > *
> > - * If @a path does not exist in its final component, set @a *kind to
> > - * @c svn_node_none.
> > + * If @a path does not exist, set @a *kind to @c svn_node_none.
> > *
> > - * If intermediate directories on the way to @a path don't exist, return
> > - * an error, with @a *kind's value undefined.
> > + * If @a path exists but is none of the above, set @a *kind to @c
> > + * svn_node_unknown.
> > *
> > - * Otherwise, set @a *kind to @c svn_node_unknown.
> > + * If unable to determine @a path's kind, return an error, with @a *kind's
> > + * value undefined.
> > *
> > * Use @a pool for temporary allocations.
> > + *
> > + * @see svn_node_kind_t
> > */
> > svn_error_t *svn_io_check_path(const char *path,
> > svn_node_kind_t *kind,
> >
> >
> > Daniel
> >
> >> and leaves the value of orig_kind uninitialized.
> >>
> >> In svn_client__make_local_parents we cannot assume that the intermediate
> >> directories exist; therefore, the above call might raise an error.
> >> However, 'mkdir --parents' works in HEAD; an error is not raised.
> >>
> >>
> >> This leaves me with two questions:
> >>
> >> * Should we be calling svn_io_check_path in
> >> svn_client__make_local_parents ?
> >>
> >> * Is svn_io_check_path's docstring still wrong? (when it says errors
> >> would be returned)
> >>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> > For additional commands, e-mail: dev-help@subversion.tigris.org
> >
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.2.6 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
> iD8DBQFIHxnO3WHvyO0YTCwRAjO6AKCUaYo8Ot7myM8+Bx0o/330S7qpfQCdHvTW
> fcJbF+aI1SLHqLu6c4M54aU=
> =rpHf
> -----END PGP SIGNATURE-----
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org