subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Fuhrmann <stefan.fuhrm...@wandisco.com>
Subject Re: svn commit: r1616338 - /subversion/trunk/subversion/libsvn_fs_fs/fs.c
Date Wed, 13 Aug 2014 08:45:28 GMT
On Mon, Aug 11, 2014 at 6:41 PM, Stefan Sperling <stsp@elego.de> wrote:

> On Mon, Aug 11, 2014 at 06:35:09PM +0200, Stefan Fuhrmann wrote:
> > On Wed, Aug 6, 2014 at 10:59 PM, Stefan Sperling <stsp@elego.de> wrote:
> > > > @@ -245,6 +245,16 @@ initialize_fs_struct(svn_fs_t *fs)
> > > >    return SVN_NO_ERROR;
> > > >  }
> > > >
> > > > +/* Reset vtable and fsap_data fields in FS such that the FS is
> basically
> > > > + * closed now.  Note that FS must not hold locks when you call
> this. */
> > > > +static svn_error_t *
> > > > +uninitialize_fs_struct(svn_fs_t *fs)
> > >
> > > I'd suggest to declare this as:
> > >
> > > static void
> > > uninitialize_fs_struct(svn_fs_t *fs)
> > >
> >
> > Thanks for the review.
> >
> > You are right, I might change that to plain void but I'll keep
> > the error return type for now, just for symmetry with the init
> > code. This is also a function that might have actual error
> > conditions in later releases and a missed caller update would
> > than create an error leak (and probably worse).
> >
> > -- Stefan^2.
>
> By that logic we shouldn't have any void functions ;-)
>
> I'm still in favour of changing it because it makes the code look
> untidy from my point of view. But whatever...
>

Alright, alright :), changed in r1617688.

-- Stefan^2.

Mime
View raw message