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 2002/12/03 18:23:34 UTC
svn_fs__parse_skel prototype question
Hello
While doing an "enable lots of warnings" build I came across
svn_fs__parse_skel in libsvn_fs/util/skel.h, which looks like
/* Parse the LEN bytes at DATA as the concrete representation of a
skel, and return a skel object allocated from POOL describing its
contents. If the data is not a properly-formed SKEL object, return
zero.
The returned skel objects point into the block indicated by DATA
and LEN; we don't copy the contents.
You'd think that DATA would be a `const char *', but we want to
create `skel' structures that point into it, and a skel's DATA
pointer shouldn't be a `const char *', since that would constrain
how the caller can use the structure. We only want to say that
*we* won't change it --- we don't want to prevent the caller from
changing it --- but C's type system doesn't allow us to say that. */
skel_t *svn_fs__parse_skel (char *data, apr_size_t len,
apr_pool_t *pool);
I don't understand the comment about not using 'const char*' for DATA.
If svn_fs__parse_skel is not going to change DATA then 'const char*'
is exactly what is wanted, and making it const in no way constrains
the callers.
The comment has been there since rev 1. Can anyone explain what the
author meant? If not, I'll change it.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn_fs__parse_skel prototype question
Posted by Philip Martin <ph...@codematters.co.uk>.
Colin Watson <cj...@flatline.org.uk> writes:
> On Tue, Dec 03, 2002 at 01:14:29PM -0800, Greg Stein wrote:
> > On Tue, Dec 03, 2002 at 02:10:01PM -0500, Greg Hudson wrote:
> > > On Tue, 2002-12-03 at 13:23, Philip Martin wrote:
> > > > I don't understand the comment about not using 'const char*' for DATA.
> > > > If svn_fs__parse_skel is not going to change DATA then 'const char*'
> > > > is exactly what is wanted, and making it const in no way constrains
> > > > the callers.
> > >
> > > The created skel will point into DATA, not a copy of it. Data pointed
> > > to by skels can be modified. (I don't know if we ever do modify data
> > > pointed to by skels, and I'd say our lives would be safer if skels were
> > > immutable, but that's not how things are right now.)
> >
> > I agree, and wanted to do this change a long while ago. I think that at one
> > point, I *did* change skel->data to be constant, but Jim backed it out.
>
> My copy of libsvn_fs/util/skel.h has 'const char *data' in skel_t, and
> it seems to have been that way since libsvn_fs/skel.h in r1. Am I
> missing something?
Whatever it is, I'm missing it as well :)
I'm currently using a patch that changes the DATA parameter to
svn_fs__parse_skel into 'const char *', and removes some casts that
are then no longer needed. It all seems to work...
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn_fs__parse_skel prototype question
Posted by Colin Watson <cj...@flatline.org.uk>.
On Tue, Dec 03, 2002 at 01:14:29PM -0800, Greg Stein wrote:
> On Tue, Dec 03, 2002 at 02:10:01PM -0500, Greg Hudson wrote:
> > On Tue, 2002-12-03 at 13:23, Philip Martin wrote:
> > > I don't understand the comment about not using 'const char*' for DATA.
> > > If svn_fs__parse_skel is not going to change DATA then 'const char*'
> > > is exactly what is wanted, and making it const in no way constrains
> > > the callers.
> >
> > The created skel will point into DATA, not a copy of it. Data pointed
> > to by skels can be modified. (I don't know if we ever do modify data
> > pointed to by skels, and I'd say our lives would be safer if skels were
> > immutable, but that's not how things are right now.)
>
> I agree, and wanted to do this change a long while ago. I think that at one
> point, I *did* change skel->data to be constant, but Jim backed it out.
My copy of libsvn_fs/util/skel.h has 'const char *data' in skel_t, and
it seems to have been that way since libsvn_fs/skel.h in r1. Am I
missing something?
--
Colin Watson [cjwatson@flatline.org.uk]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn_fs__parse_skel prototype question
Posted by Greg Stein <gs...@lyra.org>.
On Tue, Dec 03, 2002 at 02:10:01PM -0500, Greg Hudson wrote:
> On Tue, 2002-12-03 at 13:23, Philip Martin wrote:
> > I don't understand the comment about not using 'const char*' for DATA.
> > If svn_fs__parse_skel is not going to change DATA then 'const char*'
> > is exactly what is wanted, and making it const in no way constrains
> > the callers.
>
> The created skel will point into DATA, not a copy of it. Data pointed
> to by skels can be modified. (I don't know if we ever do modify data
> pointed to by skels, and I'd say our lives would be safer if skels were
> immutable, but that's not how things are right now.)
I agree, and wanted to do this change a long while ago. I think that at one
point, I *did* change skel->data to be constant, but Jim backed it out.
Whatever the case, yes: we should change skel->data to be 'const char *' and
then ripple the change outwards. Note that there are a lot of casts to
'char *' throughout the code to satisfy functions that take char*, but we
only have const data. [those casts will need to go away]
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn_fs__parse_skel prototype question
Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2002-12-03 at 13:23, Philip Martin wrote:
> I don't understand the comment about not using 'const char*' for DATA.
> If svn_fs__parse_skel is not going to change DATA then 'const char*'
> is exactly what is wanted, and making it const in no way constrains
> the callers.
The created skel will point into DATA, not a copy of it. Data pointed
to by skels can be modified. (I don't know if we ever do modify data
pointed to by skels, and I'd say our lives would be safer if skels were
immutable, but that's not how things are right now.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org