Re: svn commit: r1453590 - in /subversion/branches/fsfs-format7/subversion: include/private/svn_string_private.h libsvn_subr/string.c

On Thu, Mar 7, 2013 at 3:52 PM, C. Michael Pilato <cmpilato_at_collab.net>wrote:

> On 03/06/2013 05:15 PM, stefan2_at_apache.org wrote:
> > Author: stefan2
> > Date: Wed Mar 6 22:15:36 2013
> > New Revision: 1453590
> >
> > URL: http://svn.apache.org/r1453590
> > Log:
> > On the fsfs-format7 branch: Introduce uint <-> string conversion
> > functions that operate on base36 strings as they are used within
> > our node, copy and txn ids.
> >
> > This is the first of a long list of commits that tries to break
> > down the 5k lines patch I wrote to replace the internal usage of
> > string IDs with numerical IDs.
> >
> > * subversion/include/private/svn_string_private.h
> > (svn__ui64tobase36,
> > svn__base36toui64): declare new private API functions
> >
> > * subversion/libsvn_subr/string.c
> > (svn__ui64tobase36,
> > svn__base36toui64): implement them
>
> Maybe I missed it, but I didn't see anything by way of overflow protection
> here in the base36 -> uint64 direction. It may be an (*extremely*)
> unlikely
> scenario, but as a guy reading this code down the road, it'd be nice to see
> -- if only in a comment -- that it was something we've thought about.
>

Thanks for the review!

There was an actual buffer overflow condition that's
now fixed (r1454307). Same revision documents that
the results are "undefined" in case of an overflow.

-- Stefan^2.

-- 
Certified & Supported Apache Subversion Downloads:
*
http://www.wandisco.com/subversion/download
*