[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r17844 - trunk/subversion/libsvn_wc

From: Paul Burba <paulb_at_softlanding.com>
Date: 2005-12-19 22:27:12 CET

Malcolm Rowe <malcolm-svn-dev@farside.org.uk> wrote on 12/19/2005 02:55:31
PM:

> On Mon, Dec 19, 2005 at 01:12:58PM -0500, Paul Burba wrote:
> > > > --- trunk/subversion/libsvn_wc/status.c (original)
> > > > +++ trunk/subversion/libsvn_wc/status.c Mon Dec 19 09:00:09 2005
> > > > @@ -1054,7 +1054,7 @@
> > > > {
> > > > struct dir_baton *b = baton;
> > > > if (b->url)
> > > > - statstruct->url = b->url;
> > > > + statstruct->url = apr_pstrdup (pool, b->url);
> > >
> > > At this point, if b->url was NULL, statstruct->url will retain the
same
> > > value as it had from the call to svn_wc_status2(), and the same is
true
> > > of ood_last_cmt_author. Is this correct in all four cases (the two
> > here,
> > > and the two in the file_baton case), or should we be setting either
of
> > > the fields to NULL if the corresponding baton field is NULL?
> >
> > I don't think that is necessary since svn_wc_status2() calls
> > assemble_status(), initializing ood_last_cmt_author and url to null.
> >
>
> Okay.. Ah, svn_wc_status2() doesn't populate any of the OOD information.
> Fair enough, though it'd be nice if we documented that explicitly,
> however obvious it might be.
>
> I think you're wrong about the URL field though, since assemble_status()
> will initialise that to entry->url, which, as far as I can see, should
> always be populated.

Doh, yes, you are mostly right (which sounds better than "I am mostly
wrong"). I say "mostly" because if an item is out of date because it has
been added to the repos but doesn't exist in the WC, then entry itself is
null and assemble_status initializes url to null. The comment at the
start of assemble_status() is a bit misleading re this fact:
 
   ENTRY may be null, for non-versioned entities. In this case, we
   will assemble a special status structure item which implies a
   non-versioned thing.

Anyway, I was looking at that section of the code and then scanned down to
the end of the function and misread the line

   stat->url = (entry->url ? entry->url : NULL);

> Perhaps b->url is always the same as
> statstruct->url? (in which case this wouldn't matter)
 
> > > [I could imagine that perhaps b->url is never expected to be NULL if
> > > statstruct->url was non-NULL,
> >
> > Sorry, I don't follow you on that...did you mean that the other way
> > around?
> >
>
> No, I meant it that way round, I'm fairly sure :-)
>
> I was hypothesising that perhaps we were relying on an assumption that
> b->url could only be NULL if statstruct->url was also NULL, and hence
> the code shown above would be fine.

I think what's puzzling me is under what scenario b->url could *ever* be
null?
 
> Regards,
> Malcolm

_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. and SoftLanding Europe Plc by IBM Email Security Management Services powered by MessageLabs.
_____________________________________________________________________________

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Dec 19 22:52:12 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.