You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Madan U S <ma...@collab.net> on 2006/12/28 05:53:15 UTC
RE: svn commit: r22818 - trunk/subversion/bindings/swig/ruby/libsvn_swig_ruby
> -----Original Message-----
> From: kou@tigris.org [mailto:kou@tigris.org]
> Sent: Thu 12/28/2006 11:09 AM
> To: svn@subversion.tigris.org
> Subject: svn commit: r22818 - trunk/subversion/bindings/swig/ruby/libsvn_swig_ruby
>
> Author: kou
> Date: Wed Dec 27 21:39:50 2006
> New Revision: 22818
>
> Log:
> Use SIZEOF_LONG_LONG instead of sizeof(long long).
Please mention macro'ing of AI642NUM too in the above log summary.
>
> * subversion/bindings/swig/ruby/libsvn_swig_ruby/swigutil_rb.c
> (AOFF2NUM, AI642NUM): New macro.
> (ra_callbacks_progress_func): Use AOFF2NUM.
> (svn_swig_rb_client_blame_receiver_func): Use AI642NUM.
>
> Suggested by: Joe Swatosh
>
>
> Modified:
> trunk/subversion/bindings/swig/ruby/libsvn_swig_ruby/swigutil_rb.c
>
> Modified: trunk/subversion/bindings/swig/ruby/libsvn_swig_ruby/swigutil_rb.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/bindings/swig/ruby/libsvn_swig_ruby/swigutil_rb.c?pathrev=22818&r1=22817&r2=22818
> ==============================================================================
> --- trunk/subversion/bindings/swig/ruby/libsvn_swig_ruby/swigutil_rb.c (original)
> +++ trunk/subversion/bindings/swig/ruby/libsvn_swig_ruby/swigutil_rb.c Wed Dec 27 21:39:50 2006
> @@ -17,6 +17,15 @@
> #include "svn_utf.h"
>
>
> +#define AOFF2NUM(num) \
> + (sizeof(apr_off_t) == SIZEOF_LONG_LONG ? LL2NUM(num) : LONG2NUM(num))
> +
> +#if SIZEOF_LONG_LONG == 8
The current equivalent of this comparison is:
sizeof(apr_int64_t) == sizeof(long long)
You have hardcoded 8 instead of apr_int64_t. Is this intentional? Why? If so, I think you should make a note of it in the log.
Regards,
Madan.
Re: svn commit: r22818 - trunk/subversion/bindings/swig/ruby/libsvn_swig_ruby
Posted by Kamesh Jayachandran <ka...@collab.net>.
>
> > > > +#if SIZEOF_LONG_LONG == 8
> > >
> > > The current equivalent of this comparison is:
> > > sizeof(apr_int64_t) == sizeof(long long)
> > >
> > > You have hardcoded 8 instead of apr_int64_t. Is this intentional?
> Why? If so, I think you should make a note of it in the log.
> >
> > apr_int64_t's size must be 64bits and 64bits is 8bytes.
> > I think sizeof(apr_int64_t) == 8 is clear. Should I add a note?
>
> I understand the correctness of the value, but isnt it better to
> maintain it as sizeof(apr_int64_t) instead. It would be more generic,
> and would still be applicable if (in a rare case) the definition of
> apr_int64_t changes.
>
In some sense, I would say that 'apr_int64_t' is also kind of hardcoding
(see 64 in it). Any change that would make sizeof(apr_int64_t) to other
than 8 is next to impossible as it is part of apr(which should promise
portable runtime, which is not possible by changing the sizeof
apr_int64_t or some such).
So we can better live with
#if SIZEOF_LONG_LONG == 8
With regards
Kamesh Jayachandran
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: svn commit: r22818 - trunk/subversion/bindings/swig/ruby/libsvn_swig_ruby
Posted by Madan U S <ma...@collab.net>.
> -----Original Message-----
> From: koutou@gmail.com on behalf of Kouhei Sutou
> Sent: Thu 12/28/2006 11:39 AM
> To: Madan U S
> Cc: dev@subversion.tigris.org
> Subject: Re: svn commit: r22818 - trunk/subversion/bindings/swig/ruby/libsvn_swig_ruby
>
> Hi,
>
> 2006/12/28, Madan U S <ma...@collab.net>:
[snip]
> > > +#if SIZEOF_LONG_LONG == 8
> >
> > The current equivalent of this comparison is:
> > sizeof(apr_int64_t) == sizeof(long long)
> >
> > You have hardcoded 8 instead of apr_int64_t. Is this intentional? Why? If so, I think you should make a note of it in the log.
>
> apr_int64_t's size must be 64bits and 64bits is 8bytes.
> I think sizeof(apr_int64_t) == 8 is clear. Should I add a note?
I understand the correctness of the value, but isnt it better to maintain it as sizeof(apr_int64_t) instead. It would be more generic, and would still be applicable if (in a rare case) the definition of apr_int64_t changes.
But honestly, I dont know enough about this part of the code to comment that we *must not* hardcode this. So, its your pick.
Thanks for the immediate reply :)
Regards,
Madan.
Re: svn commit: r22818 - trunk/subversion/bindings/swig/ruby/libsvn_swig_ruby
Posted by Kouhei Sutou <ko...@cozmixng.org>.
Hi,
2006/12/28, Madan U S <ma...@collab.net>:
> > Log:
> > Use SIZEOF_LONG_LONG instead of sizeof(long long).
>
> Please mention macro'ing of AI642NUM too in the above log summary.
OK. I did.
> > +#if SIZEOF_LONG_LONG == 8
>
> The current equivalent of this comparison is:
> sizeof(apr_int64_t) == sizeof(long long)
>
> You have hardcoded 8 instead of apr_int64_t. Is this intentional? Why? If so, I think you should make a note of it in the log.
apr_int64_t's size must be 64bits and 64bits is 8bytes.
I think sizeof(apr_int64_t) == 8 is clear. Should I add a note?
Thanks,
--
kou
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org