You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2008/09/25 01:53:01 UTC

Re: svn commit: r33267 - in trunk/subversion: include libsvn_client libsvn_subr

On Wed, Sep 24, 2008 at 9:40 AM,  <ju...@tigris.org> wrote:
>...
> +++ trunk/subversion/libsvn_client/merge.c      Wed Sep 24 09:40:13 2008        (r33267)
>...
> +compare_merge_path_t_as_paths(const void *a,
> +                              const void *b)
>  {
> -  int j = 0;
> -  *child_or_parent = NULL;
> +  svn_client__merge_path_t *child1 = *((svn_client__merge_path_t * const *) a);
> +  svn_client__merge_path_t *child2 = *((svn_client__merge_path_t * const *) b);

Don't you want something like:

  const svn_client__merge_path_t *child1 = *((const
svn_client__merge_path_t **)a);

iow, you want a const structure, not the pointer itself.

>...

Cheers,
-g

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r33267 - in trunk/subversion: include libsvn_client libsvn_subr

Posted by Julian Foad <ju...@btopenworld.com>.
On Wed, 2008-09-24 at 18:53 -0700, Greg Stein wrote:
> On Wed, Sep 24, 2008 at 9:40 AM,  <ju...@tigris.org> wrote:
> >...
> > +++ trunk/subversion/libsvn_client/merge.c      Wed Sep 24 09:40:13 2008        (r33267)
> >...
> > +compare_merge_path_t_as_paths(const void *a,
> > +                              const void *b)
> >  {
> > -  int j = 0;
> > -  *child_or_parent = NULL;
> > +  svn_client__merge_path_t *child1 = *((svn_client__merge_path_t * const *) a);
> > +  svn_client__merge_path_t *child2 = *((svn_client__merge_path_t * const *) b);
> 
> Don't you want something like:
> 
>   const svn_client__merge_path_t *child1 = *((const
> svn_client__merge_path_t **)a);
> 
> iow, you want a const structure, not the pointer itself.

Yes and no. Yes in that we don't need a writable pointer to the
structure. Adding "const" there would be correct.

However, the pointer to the pointer does still need the "const"
qualifier, and this is because we are not allowed to write through it,
as defined by the function prototype. Casting away this "const" causes
some compilers to issue a warning, even though the compiler can see that
we do not in fact try to write through it.

The correct version would be:
[[[
static int
compare_merge_path_t_as_paths(const void *a,
                              const void *b)
{
  const svn_client__merge_path_t *child1
    = *((const svn_client__merge_path_t * const *) a);
  const svn_client__merge_path_t *child2
    = *((const svn_client__merge_path_t * const *) b);

  return svn_path_compare_paths(child1->path, child2->path);
}
]]]

Committed revision 33291.

(FWIW, note that despite the look of the diff, this function was only
moved, not modified, in this change.)

Thanks.

- Julian



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org