You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@xbc.nu> on 2001/01/12 00:53:22 UTC

Re: CVS update: subversion/subversion/libsvn_fs clones-table.c dag.c txn-table.c

kfogel@tigris.org wrote:

>   User: kfogel  
>   Date: 01/01/11 13:21:08
> 
>   Modified:    subversion/libsvn_fs clones-table.c dag.c txn-table.c
>   Log:
>   Add casts away from const, to get rid of compiler warnings.
>   
>   We have const strings being passed into functions that take non-const
>   strings, but -- if I understand correctly -- we don't want those
>   functions promising the incoming data will never be changed.  After
>   all, the functions are just storing the data directly (non-copying)
>   into skels, and it would be too restrictive if all skels had to
>   promise never to modify the data in their strings.
>   
>   If this is wrong, and skels can make such a promise, then we should do
>   this change the other way, adding const to the string parameters of
>   svn_fs__str_atom(), svn_fs__parse_skel(), and svn_fs__str_to_dbt().

Well, skels don't change their contents directly right now, and there's 
no API for that. I'm +1e5 for constifying those parameters, and also the 
data pointers within the skels (not the child and sibling pointers, of 
course).

Anyway, if we're going to point from skels into constant strings, we'd 
better make sure those pointers are to const, or we'll regret it 
someday. I think the casts in this case are dangerous. If we can't agree 
to constify the pointers, then I'd rather see we leave the warnings 
rather than add the casts. At the very least, the poor guy that's gonna 
have to fix the segfaults will know where to look.


-- 
Brane �ibej
    home:   <br...@xbc.nu>             http://www.xbc.nu/brane/
    work:   <br...@hermes.si>   http://www.hermes-softlab.com/
     ACM:   <br...@acm.org>            http://www.acm.org/

Re: CVS update: subversion/subversion/libsvn_fs clones-table.c dag.c txn-table.c

Posted by Karl Fogel <kf...@galois.collab.net>.
Karl Fogel <kf...@galois.collab.net> writes:
> Greg Stein <gs...@lyra.org> writes:
> > btw, note that JimB agreed that making the string data const was probably
> > quite fine. He wasn't so sure about the child/sibling pointers.
> 
> Ah, okay.  (I had thought JimB had some reason to leave them mutable.)

Waitaminnit here. :-)  In skel.h, I see the following comment:

   /* [...]

      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'd like to hear from JimB that this comment is obsolete before I
revert the change.

-K

 
> > On Fri, Jan 12, 2001 at 02:20:48AM -0800, Greg Stein wrote:
> > > Um. What he said.
> > > 
> > > [ as if you couldn't guess that's what I'd say... :-) ]
> > > 
> > > Cheers,
> > > -g
> > > 
> > > On Fri, Jan 12, 2001 at 01:53:22AM +0100, Branko Cibej wrote:
> > > > kfogel@tigris.org wrote:
> > > > 
> > > > >   User: kfogel  
> > > > >   Date: 01/01/11 13:21:08
> > > > > 
> > > > >   Modified:    subversion/libsvn_fs clones-table.c dag.c txn-table.c
> > > > >   Log:
> > > > >   Add casts away from const, to get rid of compiler warnings.
> > > > >   
> > > > >   We have const strings being passed into functions that take non-const
> > > > >   strings, but -- if I understand correctly -- we don't want those
> > > > >   functions promising the incoming data will never be changed.  After
> > > > >   all, the functions are just storing the data directly (non-copying)
> > > > >   into skels, and it would be too restrictive if all skels had to
> > > > >   promise never to modify the data in their strings.
> > > > >   
> > > > >   If this is wrong, and skels can make such a promise, then we should do
> > > > >   this change the other way, adding const to the string parameters of
> > > > >   svn_fs__str_atom(), svn_fs__parse_skel(), and svn_fs__str_to_dbt().
> > > > 
> > > > Well, skels don't change their contents directly right now, and there's 
> > > > no API for that. I'm +1e5 for constifying those parameters, and also the 
> > > > data pointers within the skels (not the child and sibling pointers, of 
> > > > course).
> > > > 
> > > > Anyway, if we're going to point from skels into constant strings, we'd 
> > > > better make sure those pointers are to const, or we'll regret it 
> > > > someday. I think the casts in this case are dangerous. If we can't agree 
> > > > to constify the pointers, then I'd rather see we leave the warnings 
> > > > rather than add the casts. At the very least, the poor guy that's gonna 
> > > > have to fix the segfaults will know where to look.
> > > > 
> > > > 
> > > > -- 
> > > > Brane Cibej
> > > >     home:   <br...@xbc.nu>             http://www.xbc.nu/brane/
> > > >     work:   <br...@hermes.si>   http://www.hermes-softlab.com/
> > > >      ACM:   <br...@acm.org>            http://www.acm.org/
> > > > 
> > > 
> > > -- 
> > > Greg Stein, http://www.lyra.org/
> > 
> > -- 
> > Greg Stein, http://www.lyra.org/

Re: CVS update: subversion/subversion/libsvn_fs clones-table.c dag.c txn-table.c

Posted by Karl Fogel <kf...@galois.collab.net>.
Greg Stein <gs...@lyra.org> writes:
> btw, note that JimB agreed that making the string data const was probably
> quite fine. He wasn't so sure about the child/sibling pointers.

Ah, okay.  (I had thought JimB had some reason to leave them mutable.)

I'll undo the change and make it go the other way, then.

-K

> [ I think those *can* be, but am happy to defer that question for now ]
> 
> Cheers,
> -g
> 
> On Fri, Jan 12, 2001 at 02:20:48AM -0800, Greg Stein wrote:
> > Um. What he said.
> > 
> > [ as if you couldn't guess that's what I'd say... :-) ]
> > 
> > Cheers,
> > -g
> > 
> > On Fri, Jan 12, 2001 at 01:53:22AM +0100, Branko Cibej wrote:
> > > kfogel@tigris.org wrote:
> > > 
> > > >   User: kfogel  
> > > >   Date: 01/01/11 13:21:08
> > > > 
> > > >   Modified:    subversion/libsvn_fs clones-table.c dag.c txn-table.c
> > > >   Log:
> > > >   Add casts away from const, to get rid of compiler warnings.
> > > >   
> > > >   We have const strings being passed into functions that take non-const
> > > >   strings, but -- if I understand correctly -- we don't want those
> > > >   functions promising the incoming data will never be changed.  After
> > > >   all, the functions are just storing the data directly (non-copying)
> > > >   into skels, and it would be too restrictive if all skels had to
> > > >   promise never to modify the data in their strings.
> > > >   
> > > >   If this is wrong, and skels can make such a promise, then we should do
> > > >   this change the other way, adding const to the string parameters of
> > > >   svn_fs__str_atom(), svn_fs__parse_skel(), and svn_fs__str_to_dbt().
> > > 
> > > Well, skels don't change their contents directly right now, and there's 
> > > no API for that. I'm +1e5 for constifying those parameters, and also the 
> > > data pointers within the skels (not the child and sibling pointers, of 
> > > course).
> > > 
> > > Anyway, if we're going to point from skels into constant strings, we'd 
> > > better make sure those pointers are to const, or we'll regret it 
> > > someday. I think the casts in this case are dangerous. If we can't agree 
> > > to constify the pointers, then I'd rather see we leave the warnings 
> > > rather than add the casts. At the very least, the poor guy that's gonna 
> > > have to fix the segfaults will know where to look.
> > > 
> > > 
> > > -- 
> > > Brane Cibej
> > >     home:   <br...@xbc.nu>             http://www.xbc.nu/brane/
> > >     work:   <br...@hermes.si>   http://www.hermes-softlab.com/
> > >      ACM:   <br...@acm.org>            http://www.acm.org/
> > > 
> > 
> > -- 
> > Greg Stein, http://www.lyra.org/
> 
> -- 
> Greg Stein, http://www.lyra.org/

Re: CVS update: subversion/subversion/libsvn_fs clones-table.c dag.c txn-table.c

Posted by Greg Stein <gs...@lyra.org>.
btw, note that JimB agreed that making the string data const was probably
quite fine. He wasn't so sure about the child/sibling pointers.

[ I think those *can* be, but am happy to defer that question for now ]

Cheers,
-g

On Fri, Jan 12, 2001 at 02:20:48AM -0800, Greg Stein wrote:
> Um. What he said.
> 
> [ as if you couldn't guess that's what I'd say... :-) ]
> 
> Cheers,
> -g
> 
> On Fri, Jan 12, 2001 at 01:53:22AM +0100, Branko Cibej wrote:
> > kfogel@tigris.org wrote:
> > 
> > >   User: kfogel  
> > >   Date: 01/01/11 13:21:08
> > > 
> > >   Modified:    subversion/libsvn_fs clones-table.c dag.c txn-table.c
> > >   Log:
> > >   Add casts away from const, to get rid of compiler warnings.
> > >   
> > >   We have const strings being passed into functions that take non-const
> > >   strings, but -- if I understand correctly -- we don't want those
> > >   functions promising the incoming data will never be changed.  After
> > >   all, the functions are just storing the data directly (non-copying)
> > >   into skels, and it would be too restrictive if all skels had to
> > >   promise never to modify the data in their strings.
> > >   
> > >   If this is wrong, and skels can make such a promise, then we should do
> > >   this change the other way, adding const to the string parameters of
> > >   svn_fs__str_atom(), svn_fs__parse_skel(), and svn_fs__str_to_dbt().
> > 
> > Well, skels don't change their contents directly right now, and there's 
> > no API for that. I'm +1e5 for constifying those parameters, and also the 
> > data pointers within the skels (not the child and sibling pointers, of 
> > course).
> > 
> > Anyway, if we're going to point from skels into constant strings, we'd 
> > better make sure those pointers are to const, or we'll regret it 
> > someday. I think the casts in this case are dangerous. If we can't agree 
> > to constify the pointers, then I'd rather see we leave the warnings 
> > rather than add the casts. At the very least, the poor guy that's gonna 
> > have to fix the segfaults will know where to look.
> > 
> > 
> > -- 
> > Brane Cibej
> >     home:   <br...@xbc.nu>             http://www.xbc.nu/brane/
> >     work:   <br...@hermes.si>   http://www.hermes-softlab.com/
> >      ACM:   <br...@acm.org>            http://www.acm.org/
> > 
> 
> -- 
> Greg Stein, http://www.lyra.org/

-- 
Greg Stein, http://www.lyra.org/

Re: CVS update: subversion/subversion/libsvn_fs clones-table.c dag.c txn-table.c

Posted by Greg Stein <gs...@lyra.org>.
Um. What he said.

[ as if you couldn't guess that's what I'd say... :-) ]

Cheers,
-g

On Fri, Jan 12, 2001 at 01:53:22AM +0100, Branko Cibej wrote:
> kfogel@tigris.org wrote:
> 
> >   User: kfogel  
> >   Date: 01/01/11 13:21:08
> > 
> >   Modified:    subversion/libsvn_fs clones-table.c dag.c txn-table.c
> >   Log:
> >   Add casts away from const, to get rid of compiler warnings.
> >   
> >   We have const strings being passed into functions that take non-const
> >   strings, but -- if I understand correctly -- we don't want those
> >   functions promising the incoming data will never be changed.  After
> >   all, the functions are just storing the data directly (non-copying)
> >   into skels, and it would be too restrictive if all skels had to
> >   promise never to modify the data in their strings.
> >   
> >   If this is wrong, and skels can make such a promise, then we should do
> >   this change the other way, adding const to the string parameters of
> >   svn_fs__str_atom(), svn_fs__parse_skel(), and svn_fs__str_to_dbt().
> 
> Well, skels don't change their contents directly right now, and there's 
> no API for that. I'm +1e5 for constifying those parameters, and also the 
> data pointers within the skels (not the child and sibling pointers, of 
> course).
> 
> Anyway, if we're going to point from skels into constant strings, we'd 
> better make sure those pointers are to const, or we'll regret it 
> someday. I think the casts in this case are dangerous. If we can't agree 
> to constify the pointers, then I'd rather see we leave the warnings 
> rather than add the casts. At the very least, the poor guy that's gonna 
> have to fix the segfaults will know where to look.
> 
> 
> -- 
> Brane Cibej
>     home:   <br...@xbc.nu>             http://www.xbc.nu/brane/
>     work:   <br...@hermes.si>   http://www.hermes-softlab.com/
>      ACM:   <br...@acm.org>            http://www.acm.org/
> 

-- 
Greg Stein, http://www.lyra.org/