You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Peter Samuelson <pe...@p12n.org> on 2011/07/07 19:12:29 UTC

Re: svn commit: r1143903 - in /subversion/branches/revprop-packing/subversion/libsvn_fs_fs: fs.h fs_fs.c

[hwright@apache.org]
> +      /* Update the manifest. */
> +      SVN_ERR(svn_stream_printf(manifest_stream, iterpool,
> +                                "%016" APR_OFF_T_FMT, next_offset));
> +      next_offset += finfo.size;

Bikeshed time!  I think space-padding (either %16 or %-16) would look
better than the zero-padding.  The only reason to use ASCII digits is
for human readability, after all, right?  left-alignint (%-16) also
means the scanf at the other end only has to scan 6 or so digits
instead of all 16, not that that's meaningful. (:

Also, for purely theoretical defense against ridiculousness, should we
assert(next_offset <= 9999999999999999llu) or so?  (Not sure if there's
a portable suffix for a 64-bit constant.)  Overflow is basically
impossible with today's computing and disk resources, but it would be
kinda bad if anyone managed to get it to happen.
-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/

Re: svn commit: r1143903 - in /subversion/branches/revprop-packing/subversion/libsvn_fs_fs: fs.h fs_fs.c

Posted by Hyrum K Wright <hy...@hyrumwright.org>.
Side comment: either of you should feel free to add these additions to
whatever is on the branch.  I may have created it, but I don't
consider it anything close to private.  Please feel free to hack away.

-Hyrum

On Thu, Jul 7, 2011 at 12:53 PM, Daniel Shahaf <da...@elego.de> wrote:
> I'm indifferent as to alignment, left alignment sounds fine as it's
> easier for parsing and grepping.
>
> Width: IMO 20, not 16, since offsets are 64-bit.  (And there is no need
> to be a power of 2, because (a) we use atomic move-into-place without
> in-place edits, and (b) the sequence number is currently supposed to be
> at the start, so it'd throw off the block alignment anyway.)
>
> Overflow: yes, we should check for that, at write time or at read time.
> Or both.  I think svn__atoui64() take care of that for the 'read' end...
>
> Peter Samuelson wrote on Thu, Jul 07, 2011 at 12:12:29 -0500:
>>
>> [hwright@apache.org]
>> > +      /* Update the manifest. */
>> > +      SVN_ERR(svn_stream_printf(manifest_stream, iterpool,
>> > +                                "%016" APR_OFF_T_FMT, next_offset));
>> > +      next_offset += finfo.size;
>>
>> Bikeshed time!  I think space-padding (either %16 or %-16) would look
>> better than the zero-padding.  The only reason to use ASCII digits is
>> for human readability, after all, right?  left-alignint (%-16) also
>> means the scanf at the other end only has to scan 6 or so digits
>> instead of all 16, not that that's meaningful. (:
>>
>> Also, for purely theoretical defense against ridiculousness, should we
>> assert(next_offset <= 9999999999999999llu) or so?  (Not sure if there's
>> a portable suffix for a 64-bit constant.)  Overflow is basically
>> impossible with today's computing and disk resources, but it would be
>> kinda bad if anyone managed to get it to happen.
>> --
>> Peter Samuelson | org-tld!p12n!peter | http://p12n.org/
>

Re: svn commit: r1143903 - in /subversion/branches/revprop-packing/subversion/libsvn_fs_fs: fs.h fs_fs.c

Posted by Daniel Shahaf <da...@elego.de>.
I'm indifferent as to alignment, left alignment sounds fine as it's
easier for parsing and grepping.

Width: IMO 20, not 16, since offsets are 64-bit.  (And there is no need
to be a power of 2, because (a) we use atomic move-into-place without
in-place edits, and (b) the sequence number is currently supposed to be
at the start, so it'd throw off the block alignment anyway.)

Overflow: yes, we should check for that, at write time or at read time.
Or both.  I think svn__atoui64() take care of that for the 'read' end...

Peter Samuelson wrote on Thu, Jul 07, 2011 at 12:12:29 -0500:
> 
> [hwright@apache.org]
> > +      /* Update the manifest. */
> > +      SVN_ERR(svn_stream_printf(manifest_stream, iterpool,
> > +                                "%016" APR_OFF_T_FMT, next_offset));
> > +      next_offset += finfo.size;
> 
> Bikeshed time!  I think space-padding (either %16 or %-16) would look
> better than the zero-padding.  The only reason to use ASCII digits is
> for human readability, after all, right?  left-alignint (%-16) also
> means the scanf at the other end only has to scan 6 or so digits
> instead of all 16, not that that's meaningful. (:
> 
> Also, for purely theoretical defense against ridiculousness, should we
> assert(next_offset <= 9999999999999999llu) or so?  (Not sure if there's
> a portable suffix for a 64-bit constant.)  Overflow is basically
> impossible with today's computing and disk resources, but it would be
> kinda bad if anyone managed to get it to happen.
> -- 
> Peter Samuelson | org-tld!p12n!peter | http://p12n.org/