You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Michael Pilato" <cm...@collab.net> on 2013/03/07 15:52:24 UTC
Re: svn commit: r1453590 - in /subversion/branches/fsfs-format7/subversion:
include/private/svn_string_private.h libsvn_subr/string.c
On 03/06/2013 05:15 PM, stefan2@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.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: svn commit: r1453590 - in /subversion/branches/fsfs-format7/subversion:
include/private/svn_string_private.h libsvn_subr/string.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Thu, Mar 7, 2013 at 3:52 PM, C. Michael Pilato <cm...@collab.net>wrote:
> On 03/06/2013 05:15 PM, stefan2@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
*