Re: svn commit: r12721 - in branches/locking/subversion: include libsvn_fs_fs

On Fri, 14 Jan 2005 [email protected] wrote:

> Author: fitz
> Date: Fri Jan 14 00:23:58 2005
> New Revision: 12721
>
> Modified: branches/locking/subversion/include/svn_error_codes.h
> Url: http://svn.collab.net/viewcvs/svn/branches/locking/subversion/include/svn_error_codes.h?view=diff&rev=12721&p1=branches/locking/subversion/include/svn_error_codes.h&r1=12720&p2=branches/locking/subversion/include/svn_error_codes.h&r2=12721
> ==============================================================================
> --- branches/locking/subversion/include/svn_error_codes.h	(original)
> +++ branches/locking/subversion/include/svn_error_codes.h	Fri Jan 14 00:23:58 2005
> @@ -545,6 +545,10 @@
>                SVN_ERR_FS_CATEGORY_START + 41,
>                "Item is out of date")
>
> +  SVN_ERRDEF (SVN_ERR_FS_INVALID_LOCKFILE,
> +              SVN_ERR_FS_CATEGORY_START + 42,
> +              "Item is out of date")
> +
>
We already have SVN_FS_CORRUPT.  I think that's the right error for this
situation.  Also, the error message is bogus.

> Modified: branches/locking/subversion/libsvn_fs_fs/lock.c
> Url: http://svn.collab.net/viewcvs/svn/branches/locking/subversion/libsvn_fs_fs/lock.c?view=diff&rev=12721&p1=branches/locking/subversion/libsvn_fs_fs/lock.c&r1=12720&p2=branches/locking/subversion/libsvn_fs_fs/lock.c&r2=12721
> ==============================================================================
> --- branches/locking/subversion/libsvn_fs_fs/lock.c	(original)
> +++ branches/locking/subversion/libsvn_fs_fs/lock.c	Fri Jan 14 00:23:58 2005
> @@ -525,27 +525,35 @@
>    lock = apr_palloc (pool, sizeof (*lock));
>
>    val = hash_fetch (hash, CREATION_DATE_KEY, pool);
> -  if (val)
> -    svn_time_from_cstring (&(lock->creation_date), val, pool);
> +  if (!val)
> +    return svn_fs_fs__err_invalid_lockfile (fs, CREATION_DATE_KEY, abs_path);
> +  svn_time_from_cstring (&(lock->creation_date), val, pool);

Ignored error.

>
>    val = hash_fetch (hash, EXPIRATION_DATE_KEY, pool);
> -  if (val)
> +  if (!val || val == 0) /* No expiration date. */

"val == 0" is redundant and confusing.

> +    lock->expiration_date = 0;
> +  else
>      svn_time_from_cstring (&(lock->expiration_date), val, pool);
>
And this error can also feel bad being leaked.

Regards,
//Peter