You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@codematters.co.uk> on 2005/01/12 22:28:33 UTC
Re: svn commit: r12683 - branches/locking/subversion/libsvn_fs_fs
fitz@tigris.org writes:
> Author: fitz
> Date: Wed Jan 12 13:18:44 2005
> New Revision: 12683
> --- branches/locking/subversion/libsvn_fs_fs/lock.c (original)
> +++ branches/locking/subversion/libsvn_fs_fs/lock.c Wed Jan 12 13:18:44 2005
> + err = svn_io_file_read_full (fd, buf, nbytes, &nbytes, pool);
> +
> + if (err && APR_STATUS_IS_EOF (err->apr_err))
> {
> - apr_file_close(fd);
> - return svn_error_wrap_apr (status,
> - _("Error reading lock entries file '%s.'"),
> - path);
> + svn_error_clear (err);
> + break;
> + }
> + if (err)
> + {
> + SVN_ERR (svn_io_file_close (fd, pool));
If svn_io_file_close fails then err leaks.
> + return err;
> }
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r12683 - branches/locking/subversion/libsvn_fs_fs
Posted by "Brian W. Fitzpatrick" <fi...@collab.net>.
On Jan 12, 2005, at 5:19 PM, Philip Martin wrote:
> "Brian W. Fitzpatrick" <fi...@collab.net> writes:
>
>>>> + if (err)
>>>> + {
>>>> + SVN_ERR (svn_io_file_close (fd, pool));
>>>
>>> If svn_io_file_close fails then err leaks.
>>
>> So just call svn_error_clear before svn_io_file_close?
>
> No, because if svn_io_file_close doesn't fail then err is returned.
> Use
> svn_error_clear (svn_io_file_close (fd, pool));
Thanks. In any case, I've rejiggered the code to have the caller close
the file descriptor, so this is now moot.
> While you are here, there is a bug in read_lock_from_abs_path(). It
> does
>
> lock = apr_palloc (pool, sizeof (*lock));
>
> val = hash_fetch (hash, PATH_KEY, pool);
> if (val)
> lock->path = val;
>
> and if val == NULL lock->path is unitialised. The same problem
> affects the other lock members as well.
I'll fix that. We should probably bail if anything we get from the
hash is null.
Thanks,
-Fitz
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r12683 - branches/locking/subversion/libsvn_fs_fs
Posted by Philip Martin <ph...@codematters.co.uk>.
"Brian W. Fitzpatrick" <fi...@collab.net> writes:
>> > + if (err)
>> > + {
>> > + SVN_ERR (svn_io_file_close (fd, pool));
>>
>> If svn_io_file_close fails then err leaks.
>
> So just call svn_error_clear before svn_io_file_close?
No, because if svn_io_file_close doesn't fail then err is returned.
Use
svn_error_clear (svn_io_file_close (fd, pool));
While you are here, there is a bug in read_lock_from_abs_path(). It
does
lock = apr_palloc (pool, sizeof (*lock));
val = hash_fetch (hash, PATH_KEY, pool);
if (val)
lock->path = val;
and if val == NULL lock->path is unitialised. The same problem
affects the other lock members as well.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r12683 - branches/locking/subversion/libsvn_fs_fs
Posted by "Brian W. Fitzpatrick" <fi...@collab.net>.
On Wed, 2005-01-12 at 16:28, Philip Martin wrote:
> fitz@tigris.org writes:
>
> > Author: fitz
> > Date: Wed Jan 12 13:18:44 2005
> > New Revision: 12683
>
> > --- branches/locking/subversion/libsvn_fs_fs/lock.c (original)
> > +++ branches/locking/subversion/libsvn_fs_fs/lock.c Wed Jan 12 13:18:44 2005
>
> > + err = svn_io_file_read_full (fd, buf, nbytes, &nbytes, pool);
> > +
> > + if (err && APR_STATUS_IS_EOF (err->apr_err))
> > {
> > - apr_file_close(fd);
> > - return svn_error_wrap_apr (status,
> > - _("Error reading lock entries file '%s.'"),
> > - path);
> > + svn_error_clear (err);
> > + break;
> > + }
> > + if (err)
> > + {
> > + SVN_ERR (svn_io_file_close (fd, pool));
>
> If svn_io_file_close fails then err leaks.
So just call svn_error_clear before svn_io_file_close?
-Fitz
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org