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 2011/05/23 18:22:55 UTC

Re: svn commit: r1126553 - in /subversion/trunk/subversion: libsvn_wc/upgrade.c tests/cmdline/upgrade_tests.py tests/cmdline/upgrade_tests_data/format_28.tar.bz2

On May 23, 2011 11:57 AM, <ju...@apache.org> wrote:
>...
was 40 hex digits. */
> -  if (finfo->filetype == APR_REG && strlen(path) == 40)
> +  if (finfo->filetype == APR_REG
> +      && (strlen(svn_dirent_basename(abspath, pool))
> +          ==

On my phone, so I can't tell, but ... can basename() take NULL for the pool?

Re: svn commit: r1126553 - in /subversion/trunk/subversion: libsvn_wc/upgrade.c tests/cmdline/upgrade_tests.py tests/cmdline/upgrade_tests_data/format_28.tar.bz2

Posted by Julian Foad <ju...@wandisco.com>.
On Mon, 2011-05-23 at 13:16 -0400, Greg Stein wrote:
> On May 23, 2011 12:48 PM, "Julian Foad" <ju...@wandisco.com> wrote:
> >
> > Greg Stein wrote:
> > > On May 23, 2011 11:57 AM, <ju...@apache.org> wrote:
> > > >...
> > > was 40 hex digits. */
> > > > -  if (finfo->filetype == APR_REG && strlen(path) == 40)
> > > > +  if (finfo->filetype == APR_REG
> > > > +      && (strlen(svn_dirent_basename(abspath, pool))
> > > > +          ==
> > >
> > > On my phone, so I can't tell, but ... can basename() take NULL for the
> pool?
> >
> > Yes it can.  You thinking we should pass NULL and so squeeze a few more
> > microseconds out of the developer-only upgrade scenario?
> 
> The basename is completely transient, so there's no reason to alloc and
> copy.
> 
> That said: you're right. It was a reflexive suggestion, which is kinda moot
> given the larger context.

No sweat.  I got as far as being about to change it when I thought, Hold
on, this just isn't worth it this time.

- Julian



Re: svn commit: r1126553 - in /subversion/trunk/subversion: libsvn_wc/upgrade.c tests/cmdline/upgrade_tests.py tests/cmdline/upgrade_tests_data/format_28.tar.bz2

Posted by Greg Stein <gs...@gmail.com>.
On May 23, 2011 12:48 PM, "Julian Foad" <ju...@wandisco.com> wrote:
>
> Greg Stein wrote:
> > On May 23, 2011 11:57 AM, <ju...@apache.org> wrote:
> > >...
> > was 40 hex digits. */
> > > -  if (finfo->filetype == APR_REG && strlen(path) == 40)
> > > +  if (finfo->filetype == APR_REG
> > > +      && (strlen(svn_dirent_basename(abspath, pool))
> > > +          ==
> >
> > On my phone, so I can't tell, but ... can basename() take NULL for the
pool?
>
> Yes it can.  You thinking we should pass NULL and so squeeze a few more
> microseconds out of the developer-only upgrade scenario?

The basename is completely transient, so there's no reason to alloc and
copy.

That said: you're right. It was a reflexive suggestion, which is kinda moot
given the larger context.

Cheers,
-g

Re: svn commit: r1126553 - in /subversion/trunk/subversion: libsvn_wc/upgrade.c tests/cmdline/upgrade_tests.py tests/cmdline/upgrade_tests_data/format_28.tar.bz2

Posted by Julian Foad <ju...@wandisco.com>.
Greg Stein wrote:
> On May 23, 2011 11:57 AM, <ju...@apache.org> wrote:
> >...
> was 40 hex digits. */
> > -  if (finfo->filetype == APR_REG && strlen(path) == 40)
> > +  if (finfo->filetype == APR_REG
> > +      && (strlen(svn_dirent_basename(abspath, pool))
> > +          ==
> 
> On my phone, so I can't tell, but ... can basename() take NULL for the pool?

Yes it can.  You thinking we should pass NULL and so squeeze a few more
microseconds out of the developer-only upgrade scenario?

- Julian