Re: [PATCH] fix core and make tests pass

Philip Martin <philip@codematters.co.uk> writes:

> "Craig Peterein" <craig.peterein@myrealbox.com> writes:
>
> > + date = NULL;
> > + author = NULL;
>
> I don't think that is quite right. svn_client_commit writes to data
> and author, requiring the caller to initialise them first is ugly.

I'm getting this same segfault too in r745, every time we try to
commit to XML.

> ra_local commit does this at lines 126 to 141 in ra_local/ra_plugin.c.
> Perhaps --xml-file and ra_dav commits need something similar?

No, in the case of commits to XML, it's already silly that we're
artifically specifying a revision number on the command-line. There's
no need to worry about making this already-artificial situation start
returning more imaginary information from an imaginary RA layer.

> If we choose to initialise to null, which looks like a hack to me, it
> should be done in libsvn_client/commit.c:send_to_repos. Something
> like
>
> if (committed_date)
> *committed_date = NULL;

I'm not following you here. The command-line client creates two
uninitialzed pointers. It passes the addresses of these pointers all
the way down through libsvn_client and into the RA layer. The RA
layer allocates memory and sets the pointers.

So what's wrong if the original pointers start out as NULL? When we
commit to XML, no RA layer will ever change them, and thus the current
code at the end of libsvn_client's send_to_repos() will avoid trying
to dup their values.

Please explain the issue, I'm not sure I understand the problem.
Otherwise, I'll just go ahead and commit the NULL initializations.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:55 2006