You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kouhei Sutou <ko...@cozmixng.org> on 2005/10/16 05:26:35 UTC
Re: svn commit: r16735 - in trunk/subversion: libsvn_subr libsvn_wc
Hi,
In <20...@morbius.ch.collab.net>
> Author: djames
> New Revision: 16735
> --- trunk/subversion/libsvn_subr/constructors.c (original)
> +++ trunk/subversion/libsvn_subr/constructors.c Sat Oct 15 12:24:04 2005
> +svn_client_commit_item2_t *
> +svn_client_commit_item2_dup (const svn_client_commit_item2_t *item,
> + apr_pool_t *pool)
> +{
> + svn_client_commit_item2_t *new_item = apr_palloc (pool, sizeof (*new_item));
> +
> + *new_item = *item;
> +
> + if (new_item->path)
if (new_item->kind && new_item->path)
We need to check new_item->kind value is svn_node_none or
not. Or we initialize item->path as NULL if item->path is
not available.
> + new_item->path = apr_pstrdup (pool, new_item->path);
> +
> + if (new_item->url)
> + new_item->url = apr_pstrdup (pool, new_item->url);
if (new_item->copyfrom_url)
new_item->copyfrom_url = apr_pstrdup (pool, new_item->copyfrom_url);
We need to copy new_item->copyfrom_url.
> +
> + if (new_item->wcprop_changes)
I think we need to check new_item->wcprop_changes is
available or not. But I don't know the way...
Can we initialize item->wcprop_changes as NULL?
> + new_item->wcprop_changes = svn_prop_array_dup (new_item->wcprop_changes,
> + pool);
> +
> + return new_item;
> +}
Thanks,
--
kou
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r16735 - in trunk/subversion: libsvn_subr libsvn_wc
Posted by Kouhei Sutou <ko...@cozmixng.org>.
Hi,
In <df...@mail.gmail.com>
"Re: svn commit: r16735 - in trunk/subversion: libsvn_subr libsvn_wc" on Sun, 16 Oct 2005 02:06:07 -0400,
David James <ja...@gmail.com> wrote:
> Unlike apr_palloc, apr_pcalloc initalizes all fields to NULL, so
> item->path, item->kind, and item->wcprop_changes are initialized to
> NULL here.
I see.
David, thank you for your explain.
Cheers,
--
kou
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r16735 - in trunk/subversion: libsvn_subr libsvn_wc
Posted by David James <ja...@gmail.com>.
On 10/16/05, Kouhei Sutou <ko...@cozmixng.org> wrote:
> Hi,
>
> I'm sorry for my unclear explain.
>
> In <df...@mail.gmail.com>
> "Re: svn commit: r16735 - in trunk/subversion: libsvn_subr libsvn_wc" on Sun, 16 Oct 2005 01:45:13 -0400,
> David James <ja...@gmail.com> wrote:
>
> > > > + *new_item = *item;
> > > > +
> > > > + if (new_item->path)
> > >
> > > if (new_item->kind && new_item->path)
> > >
> > > We need to check new_item->kind value is svn_node_none or
> > > not.
> > Really? I did not know this. Why?
>
> When item->kind is svn_node_none, item->path value may be
> not initialized.
>
> For example from mkdir_urls() in libsvn_client/commit.c:
>
> item = apr_pcalloc (pool, sizeof (*item));
> item->url = svn_path_join (common, path, pool);
> item->state_flags = SVN_CLIENT_COMMIT_ITEM_ADD;
>
> item->path and item->kind are not initialized. So
> item->path's value isn't ensured. But item->kind value
> is initialized as 0 (= svn_node_none) because
> svn_node_kind_t which is item->kind type is enum type.
Unlike apr_palloc, apr_pcalloc initalizes all fields to NULL, so
item->path, item->kind, and item->wcprop_changes are initialized to
NULL here.
Cheers,
David
--
David James -- http://www.cs.toronto.edu/~james
Re: svn commit: r16735 - in trunk/subversion: libsvn_subr libsvn_wc
Posted by Kouhei Sutou <ko...@cozmixng.org>.
Hi,
I'm sorry for my unclear explain.
In <df...@mail.gmail.com>
"Re: svn commit: r16735 - in trunk/subversion: libsvn_subr libsvn_wc" on Sun, 16 Oct 2005 01:45:13 -0400,
David James <ja...@gmail.com> wrote:
> > > + *new_item = *item;
> > > +
> > > + if (new_item->path)
> >
> > if (new_item->kind && new_item->path)
> >
> > We need to check new_item->kind value is svn_node_none or
> > not.
> Really? I did not know this. Why?
When item->kind is svn_node_none, item->path value may be
not initialized.
For example from mkdir_urls() in libsvn_client/commit.c:
item = apr_pcalloc (pool, sizeof (*item));
item->url = svn_path_join (common, path, pool);
item->state_flags = SVN_CLIENT_COMMIT_ITEM_ADD;
item->path and item->kind are not initialized. So
item->path's value isn't ensured. But item->kind value
is initialized as 0 (= svn_node_none) because
svn_node_kind_t which is item->kind type is enum type.
> > Or we initialize item->path as NULL if item->path is
> > not available.
> We copy *new_item = *item, so new_item->path will be NULL if item->path is NULL.
I worry about item->path is not initialized as NULL.
> > > + if (new_item->wcprop_changes)
> >
> > I think we need to check new_item->wcprop_changes is
> > available or not. But I don't know the way...
> If new_item->wcprop_changes is not available, it would be NULL, right?
I don't think so. The reason of this is same as new_item->path.
> > Can we initialize item->wcprop_changes as NULL?
> We copy *new_item = *item, so new_item->wcprop_changes will be NULL if
> item->wcprop_changes is NULL.
I worry about item->wcprop_changes is not initialized as NULL.
Cheers,
--
kou
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r16735 - in trunk/subversion: libsvn_subr libsvn_wc
Posted by David James <ja...@gmail.com>.
On 10/16/05, Kouhei Sutou <ko...@cozmixng.org> wrote:
> Hi,
>
> In <20...@morbius.ch.collab.net>
>
> > Author: djames
> > New Revision: 16735
>
> > --- trunk/subversion/libsvn_subr/constructors.c (original)
> > +++ trunk/subversion/libsvn_subr/constructors.c Sat Oct 15 12:24:04 2005
>
> > +svn_client_commit_item2_t *
> > +svn_client_commit_item2_dup (const svn_client_commit_item2_t *item,
> > + apr_pool_t *pool)
> > +{
> > + svn_client_commit_item2_t *new_item = apr_palloc (pool, sizeof (*new_item));
> > +
> > + *new_item = *item;
> > +
> > + if (new_item->path)
>
> if (new_item->kind && new_item->path)
>
> We need to check new_item->kind value is svn_node_none or
> not.
Really? I did not know this. Why?
> Or we initialize item->path as NULL if item->path is
> not available.
We copy *new_item = *item, so new_item->path will be NULL if item->path is NULL.
> > + new_item->path = apr_pstrdup (pool, new_item->path);
> > +
> > + if (new_item->url)
> > + new_item->url = apr_pstrdup (pool, new_item->url);
>
> if (new_item->copyfrom_url)
> new_item->copyfrom_url = apr_pstrdup (pool, new_item->copyfrom_url);
>
> We need to copy new_item->copyfrom_url.
Oops. Yes, I'll add this.
> > +
> > + if (new_item->wcprop_changes)
>
> I think we need to check new_item->wcprop_changes is
> available or not. But I don't know the way...
If new_item->wcprop_changes is not available, it would be NULL, right?
> Can we initialize item->wcprop_changes as NULL?
We copy *new_item = *item, so new_item->wcprop_changes will be NULL if
item->wcprop_changes is NULL.
--
David James -- http://www.cs.toronto.edu/~james