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 2012/04/05 22:29:08 UTC
Re: svn commit: r1310047 - /subversion/trunk/subversion/libsvn_ra/compat.c
On Thu, Apr 5, 2012 at 16:15, <hw...@apache.org> wrote:
> Author: hwright
> Date: Thu Apr 5 20:15:41 2012
> New Revision: 1310047
>
> URL: http://svn.apache.org/viewvc?rev=1310047&view=rev
> Log:
> * subversion/libsvn_ra/compat.c
> (prev_log_path): Another integer-width mismatch fix.
>
> Modified:
> subversion/trunk/subversion/libsvn_ra/compat.c
>
> Modified: subversion/trunk/subversion/libsvn_ra/compat.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra/compat.c?rev=1310047&r1=1310046&r2=1310047&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra/compat.c (original)
> +++ subversion/trunk/subversion/libsvn_ra/compat.c Thu Apr 5 20:15:41 2012
> @@ -141,7 +141,7 @@ prev_log_path(const char **prev_path_p,
> svn_sort__item_t item = APR_ARRAY_IDX(paths,
> i - 1, svn_sort__item_t);
> const char *ch_path = item.key;
> - int len = strlen(ch_path);
> + size_t len = strlen(ch_path);
size_t or should this be apr_size_t?
Re: svn commit: r1310047 - /subversion/trunk/subversion/libsvn_ra/compat.c
Posted by Hyrum K Wright <hy...@wandisco.com>.
On Thu, Apr 5, 2012 at 4:43 PM, Greg Stein <gs...@gmail.com> wrote:
> On Thu, Apr 5, 2012 at 16:48, Hyrum K Wright <hy...@wandisco.com> wrote:
>> On Thu, Apr 5, 2012 at 3:29 PM, Greg Stein <gs...@gmail.com> wrote:
>>> On Thu, Apr 5, 2012 at 16:15, <hw...@apache.org> wrote:
>>>> Author: hwright
>>>> Date: Thu Apr 5 20:15:41 2012
>>>> New Revision: 1310047
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1310047&view=rev
>>>> Log:
>>>> * subversion/libsvn_ra/compat.c
>>>> (prev_log_path): Another integer-width mismatch fix.
>>>>
>>>> Modified:
>>>> subversion/trunk/subversion/libsvn_ra/compat.c
>>>>
>>>> Modified: subversion/trunk/subversion/libsvn_ra/compat.c
>>>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra/compat.c?rev=1310047&r1=1310046&r2=1310047&view=diff
>>>> ==============================================================================
>>>> --- subversion/trunk/subversion/libsvn_ra/compat.c (original)
>>>> +++ subversion/trunk/subversion/libsvn_ra/compat.c Thu Apr 5 20:15:41 2012
>>>> @@ -141,7 +141,7 @@ prev_log_path(const char **prev_path_p,
>>>> svn_sort__item_t item = APR_ARRAY_IDX(paths,
>>>> i - 1, svn_sort__item_t);
>>>> const char *ch_path = item.key;
>>>> - int len = strlen(ch_path);
>>>> + size_t len = strlen(ch_path);
>>>
>>> size_t or should this be apr_size_t?
>>
>> You ask this question *every time* one of these changes is made. :P
>>
>> And the response usually is "apr_size_t is a typedef of size_t on
>> every platform we support, and we already use a mix of the two
>> throughout our code".
>
> I count about 100 size_t usages, and over 1200 apr_size_t. I think the
> vote is for the latter :-P
I went with 'size_t' in this case since the value is the result of a
call to strlen(), a standard library function defined to return
size_t. In cases where we interact with APR, I usually favor
apr_size_t, since that's what the APIs ask for / require. I suspect
others follow this same pattern.
> Personally, I don't care much either way except for consistency. And
> so when I see a bare size_t, then I ask. I think the tendency should
> be for apr_size_t unless there is some specific reason otherwise.
I've no objection to standardization; I just don't care enough to be
the one to do it. :)
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Re: svn commit: r1310047 - /subversion/trunk/subversion/libsvn_ra/compat.c
Posted by Greg Stein <gs...@gmail.com>.
On Thu, Apr 5, 2012 at 16:48, Hyrum K Wright <hy...@wandisco.com> wrote:
> On Thu, Apr 5, 2012 at 3:29 PM, Greg Stein <gs...@gmail.com> wrote:
>> On Thu, Apr 5, 2012 at 16:15, <hw...@apache.org> wrote:
>>> Author: hwright
>>> Date: Thu Apr 5 20:15:41 2012
>>> New Revision: 1310047
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1310047&view=rev
>>> Log:
>>> * subversion/libsvn_ra/compat.c
>>> (prev_log_path): Another integer-width mismatch fix.
>>>
>>> Modified:
>>> subversion/trunk/subversion/libsvn_ra/compat.c
>>>
>>> Modified: subversion/trunk/subversion/libsvn_ra/compat.c
>>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra/compat.c?rev=1310047&r1=1310046&r2=1310047&view=diff
>>> ==============================================================================
>>> --- subversion/trunk/subversion/libsvn_ra/compat.c (original)
>>> +++ subversion/trunk/subversion/libsvn_ra/compat.c Thu Apr 5 20:15:41 2012
>>> @@ -141,7 +141,7 @@ prev_log_path(const char **prev_path_p,
>>> svn_sort__item_t item = APR_ARRAY_IDX(paths,
>>> i - 1, svn_sort__item_t);
>>> const char *ch_path = item.key;
>>> - int len = strlen(ch_path);
>>> + size_t len = strlen(ch_path);
>>
>> size_t or should this be apr_size_t?
>
> You ask this question *every time* one of these changes is made. :P
>
> And the response usually is "apr_size_t is a typedef of size_t on
> every platform we support, and we already use a mix of the two
> throughout our code".
I count about 100 size_t usages, and over 1200 apr_size_t. I think the
vote is for the latter :-P
Personally, I don't care much either way except for consistency. And
so when I see a bare size_t, then I ask. I think the tendency should
be for apr_size_t unless there is some specific reason otherwise.
Cheers,
-g
Re: svn commit: r1310047 - /subversion/trunk/subversion/libsvn_ra/compat.c
Posted by Hyrum K Wright <hy...@wandisco.com>.
On Thu, Apr 5, 2012 at 3:29 PM, Greg Stein <gs...@gmail.com> wrote:
> On Thu, Apr 5, 2012 at 16:15, <hw...@apache.org> wrote:
>> Author: hwright
>> Date: Thu Apr 5 20:15:41 2012
>> New Revision: 1310047
>>
>> URL: http://svn.apache.org/viewvc?rev=1310047&view=rev
>> Log:
>> * subversion/libsvn_ra/compat.c
>> (prev_log_path): Another integer-width mismatch fix.
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_ra/compat.c
>>
>> Modified: subversion/trunk/subversion/libsvn_ra/compat.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra/compat.c?rev=1310047&r1=1310046&r2=1310047&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_ra/compat.c (original)
>> +++ subversion/trunk/subversion/libsvn_ra/compat.c Thu Apr 5 20:15:41 2012
>> @@ -141,7 +141,7 @@ prev_log_path(const char **prev_path_p,
>> svn_sort__item_t item = APR_ARRAY_IDX(paths,
>> i - 1, svn_sort__item_t);
>> const char *ch_path = item.key;
>> - int len = strlen(ch_path);
>> + size_t len = strlen(ch_path);
>
> size_t or should this be apr_size_t?
You ask this question *every time* one of these changes is made. :P
And the response usually is "apr_size_t is a typedef of size_t on
every platform we support, and we already use a mix of the two
throughout our code".
(Of course, there's usually a bit of discussion and pedantry to get to
that conclusion, and if that's what you're after, don't let me stop
you. :) )
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/