You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@codematters.co.uk> on 2004/09/28 01:52:27 UTC

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

rooneg@tigris.org writes:

> Author: rooneg
> Date: Mon Sep 27 20:22:42 2004
> New Revision: 11155

> --- trunk/subversion/svnserve/serve.c	(original)
> +++ trunk/subversion/svnserve/serve.c	Mon Sep 27 20:22:42 2004
> @@ -887,12 +887,16 @@
>    apr_array_header_t *paths, *full_paths;
>    svn_ra_svn_item_t *elt;
>    int i;
> +  int limit;
>    log_baton_t lb;
>  
>    /* Parse the arguments. */
> -  SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "l(?r)(?r)bb", &paths,
> +  SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "l(?r)(?r)bb?n", &paths,
>                                   &start_rev, &end_rev, &changed_paths,
> -                                 &strict_node));
> +                                 &strict_node, &limit));
> +  if (limit == SVN_RA_SVN_UNSPECIFIED_NUMBER)

../svn/subversion/svnserve/serve.c: In function `log_cmd':
../svn/subversion/svnserve/serve.c:897: warning: comparison between signed and unsigned

Should limit be apr_uint64_t rather than int?

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@electricjellyfish.net> writes:

> Will casting to an apr_uint64_t in the call to
> svn_ra_svn_write_tuple be enough

Yes.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Use of 'unsigned' in interfaces [was: Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data]

Posted by Julian Foad <ju...@btopenworld.com>.
OK, I don't want to force the issue, given that we already have 'unsigned' in other APIs, but to continue the thread...

Greg Hudson wrote:
> On Fri, 2004-10-01 at 13:23, Julian Foad wrote:
> 
>>There's a big difference: 'const' is enforced whereas 'unsigned' is not.
> 
> gcc warns about the second of your examples with -W, and I think MSVC
> warns about the first (uncasted signed -> unsigned conversion), and
> modern gcc may have a warning option for that as well.

Yes, indeed they do or can.  In my first example, I mentioned in a comment that they can; I didn't mention it for the second example, which I suppose was a bit misleading.

It's not clear whether you are just picking me up on my sloppiness or whether you are implying that I'm wrong in asserting that 'unsigned' is far less useful than 'const'.  I maintain that the existence of such warnings (only when enabled or when using certain compilers) provides a distinctly lower class of safety than the errors caused by attempting to violate 'const'.  (There is another big difference in that 'const' errors apply only to the user of the 'const' pointer, whereas 'unsigned' mismatch warnings apply to both the user and the provider of the value, but I don't think that is very relevant to the discussion.)

The book "Large-Scale C++ Software Design" by John Lakos is _very_ good on both large-scale design issues and small-scale issues, many of which apply to C as well as C++, and he gives level-headed advice on appropriate use of 'unsigned' and 'short' especially in interfaces but also in implementation code.  His points are, briefly:

  "Guideline: Avoid using 'unsigned' in the interface; use 'int' instead."
  "Principle: Occasionally comments work better than trying to express an interface decision directly in the code (e.g., 'unsigned')."
  Use of 'unsigned' in the interface:
  + Prevents negative numbers from being passed?  "No. C++ allows the bit pattern to be reinterpreted silently."  (This is true for C as well.  We know that we might get a warning.)
  + Allows for the possibility for checking for negative values?  Only if you cast back to 'int'.
  + Runtime efficiency?  No difference.
  + Increase the range of positive values?  Yes, by 1 bit - rarely useful.  Risk of loss by conversion back to 'int'.
  + Likelihood of user error?  Increased, by accidental mixing with negative signed ints.
  + Encapsulation?  He says it "effectively limits the values that any implementation will accommodate, thereby reducing encapsulation".  I don't follow that.
  + (Two C++ points about it interfering with function overloading and template instantiation.)

Not trying to argue for the sake of it, just trying to advocate goodness.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

Posted by Greg Hudson <gh...@MIT.EDU>.
On Fri, 2004-10-01 at 13:23, Julian Foad wrote:
> There's a big difference: 'const' is enforced whereas 'unsigned' is not.

gcc warns about the second of your examples with -W, and I think MSVC
warns about the first (uncasted signed -> unsigned conversion), and
modern gcc may have a warning option for that as well.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

Posted by Julian Foad <ju...@btopenworld.com>.
Garrett Rooney wrote:
> Philip Martin wrote:
>> We already have functions, e.g. svn_path_remove_components, where it
>> doesn't make make sense to pass a -ve value (they don't document it
>> either) so perhaps just say "-ve values are invalid".
> 
> Why only document it when we can force the issue by declaring the 
> argument unsigned?  It seems awfully close to saying "we're never going 
> to modify this argument" instead of just making it const.

There's a big difference: 'const' is enforced whereas 'unsigned' is not.

There is a strong case for avoiding 'unsigned' integer types altogether in C programming (except for low-level bit manipulations etc.).  This has mainly to do with the fact that they hardly protect you at all from such errors because of automatic (silent) conversions from signed to unsigned, and especially because unsigned has priority when a binary operator takes one of each.

void blah(unsigned Revision);
...
int Offset = -2;
blah(Offset);  /* Generally no error or warning. (you can make some compilers warn) */
...
int Offset = ...;  /* number of revisions after (+ve) or before (-ve) Head */
unsigned Revision = HeadRevision();
if (Revision + Offset < 0) error();
/* Oops: will never error, because type of (Revision+Offset) is unsigned. */

The Java language takes this to a logical conclusion and has no unsigned types.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Philip Martin wrote:
> Garrett Rooney <ro...@electricjellyfish.net> writes:
> 
> 
>>I'm leaning towards just making it an unsigned int and moving on
>>with life myself...
> 
> 
> We already have functions, e.g. svn_path_remove_components, where it
> doesn't make make sense to pass a -ve value (they don't document it
> either) so perhaps just say "-ve values are invalid".
> 

Why only document it when we can force the issue by declaring the 
argument unsigned?  It seems awfully close to saying "we're never going 
to modify this argument" instead of just making it const.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

Posted by Philip Martin <ph...@codematters.co.uk>.
Philip Martin <ph...@codematters.co.uk> writes:

> We already have functions, e.g. svn_path_remove_components, where it
> doesn't make make sense to pass a -ve value (they don't document it
> either) so perhaps just say "-ve values are invalid".

Oops! svn_path_remove_components takes an unsigned value.  I guess
that's a good argument for the unsigned solution.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@electricjellyfish.net> writes:

> I'm leaning towards just making it an unsigned int and moving on
> with life myself...

We already have functions, e.g. svn_path_remove_components, where it
doesn't make make sense to pass a -ve value (they don't document it
either) so perhaps just say "-ve values are invalid".

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Philip Martin wrote:
> Garrett Rooney <ro...@electricjellyfish.net> writes:
> 
> 
>>I'm reasonably sure about this fixing the problems Philip pointed out,
> 
> 
> Almost ;)  svn_client.h/svn_ra.h/svn_repos.h still don't document the
> behaviour when a -ve limit is passed.

That's why I asked for more feedback on whether or not we should allow 
negative numbers or just change to an unsigned int limit argument 
instead.  I'm leaning towards just making it an unsigned int and moving 
on with life myself...

-garrett


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@electricjellyfish.net> writes:

> I'm reasonably sure about this fixing the problems Philip pointed out,

Almost ;)  svn_client.h/svn_ra.h/svn_repos.h still don't document the
behaviour when a -ve limit is passed.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Garrett Rooney wrote:

> Ok, here's a proposed patch to correct the problem on both sides of the 
> network...

I'm reasonably sure about this fixing the problems Philip pointed out, 
so I committed it in revision 11156.  If there are any more issues I'll 
take a look at them in the morning.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Garrett Rooney wrote:
> Philip Martin wrote:
> 
>> You risk having values greater than INT_MAX converted into negative
>> numbers (and the svn_repos_get_log3 documentation doesn't say what
>> happens if limit is negative :)
> 
> 
> True.  Perhaps an unsigned int would be more appropriate?  I can't think 
> of any reasonable behavior for a negative limit.
> 
>> Of course values greater than INT_MAX won't occur if you are starting
>> with an int in the first place... which raises a question about
>> ra_svn_log2: it's probably not valid to pass an int to
>> svn_ra_svn_write_tuple with an "n" format, as that will attempt to
>> retrieve 8 bytes from the varargs when only 4 bytes have been passed.

Ok, here's a proposed patch to correct the problem on both sides of the 
network...

This doesn't answer the 'signed vs unsigned' limit argument question 
though, what do people think about that?

-garrett

* subversion/libsvn_ra_svn/client.c
   (ra_svn_log2): convert limit argument into an apr_uint64_t
    before passing it in to svn_ra_svn_write_tuple.

* subversion/svnserve/serve.c
   (log_cmd): make limit an apr_uint64_t to avoid warnings about
    signed/unsigned comparison and guard against values larger
    than INT_MAX so we don't pass really big values into
    svn_repos_get_logs3.  cast limit to int before passing into
    svn_repos_get_logs3.

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Christian Stork wrote:
> On Mon, Sep 27, 2004 at 10:29:45PM -0400, Garrett Rooney wrote:
> 
>>Philip Martin wrote:
>>
>>
>>>You risk having values greater than INT_MAX converted into negative
>>>numbers (and the svn_repos_get_log3 documentation doesn't say what
>>>happens if limit is negative :)
>>
>>True.  Perhaps an unsigned int would be more appropriate?  I can't think 
>>of any reasonable behavior for a negative limit.
> 
> 
> FWIW, a negative limit could refer to the N first log messages instead
> of the N last messages.
> 

I just figured to do that you'd reverse the range.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

Posted by Christian Stork <cs...@ics.uci.edu>.
On Mon, Sep 27, 2004 at 10:29:45PM -0400, Garrett Rooney wrote:
> Philip Martin wrote:
> 
> >You risk having values greater than INT_MAX converted into negative
> >numbers (and the svn_repos_get_log3 documentation doesn't say what
> >happens if limit is negative :)
> 
> True.  Perhaps an unsigned int would be more appropriate?  I can't think 
> of any reasonable behavior for a negative limit.

FWIW, a negative limit could refer to the N first log messages instead
of the N last messages.

-- 
Chris Stork   <>  Support eff.org!  <>   http://www.ics.uci.edu/~cstork/
OpenPGP fingerprint:  B08B 602C C806 C492 D069  021E 41F3 8C8D 50F9 CA2F

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Philip Martin wrote:

> You risk having values greater than INT_MAX converted into negative
> numbers (and the svn_repos_get_log3 documentation doesn't say what
> happens if limit is negative :)

True.  Perhaps an unsigned int would be more appropriate?  I can't think 
of any reasonable behavior for a negative limit.

> Of course values greater than INT_MAX won't occur if you are starting
> with an int in the first place... which raises a question about
> ra_svn_log2: it's probably not valid to pass an int to
> svn_ra_svn_write_tuple with an "n" format, as that will attempt to
> retrieve 8 bytes from the varargs when only 4 bytes have been passed.
> 

Good point.  Will casting to an apr_uint64_t in the call to 
svn_ra_svn_write_tuple be enough to avoid that, or do we actually have 
to use a temporary variable?

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

Posted by Philip Martin <ph...@codematters.co.uk>.
Philip Martin <ph...@codematters.co.uk> writes:

> Of course values greater than INT_MAX won't occur if you are starting
> with an int in the first place... which raises a question about
> ra_svn_log2: it's probably not valid to pass an int to
> svn_ra_svn_write_tuple with an "n" format, as that will attempt to
> retrieve 8 bytes from the varargs when only 4 bytes have been passed.

$ valgrind svn log svn://localhost/repo

==31356== Conditional jump or move depends on uninitialised value(s)
==31356==    at 0x404544F7: conv_10_quad (apr_snprintf.c:397)
==31356==    by 0x404555BC: apr_vformatter (apr_snprintf.c:846)
==31356==    by 0x40465FEB: apr_pvsprintf (apr_pools.c:1604)
==31356==    by 0x402FFFAD: writebuf_printf (marshal.c:214)
==31356== 
==31356== Conditional jump or move depends on uninitialised value(s)
==31356==    at 0x404544F9: conv_10_quad (apr_snprintf.c:397)
==31356==    by 0x404555BC: apr_vformatter (apr_snprintf.c:846)
==31356==    by 0x40465FEB: apr_pvsprintf (apr_pools.c:1604)
==31356==    by 0x402FFFAD: writebuf_printf (marshal.c:214)
==31356== 
==31356== Conditional jump or move depends on uninitialised value(s)
==31356==    at 0x40467FF5: __udivdi3 (libgcc2.c:802)
==31356==    by 0x40454580: conv_10_quad (apr_snprintf.c:431)
==31356==    by 0x404555BC: apr_vformatter (apr_snprintf.c:846)
==31356==    by 0x40465FEB: apr_pvsprintf (apr_pools.c:1604)
==31356== 
==31356== Conditional jump or move depends on uninitialised value(s)
==31356==    at 0x404545B1: conv_10_quad (apr_snprintf.c:435)
==31356==    by 0x404555BC: apr_vformatter (apr_snprintf.c:846)
==31356==    by 0x40465FEB: apr_pvsprintf (apr_pools.c:1604)
==31356==    by 0x402FFFAD: writebuf_printf (marshal.c:214)
==31356== 
==31356== Conditional jump or move depends on uninitialised value(s)
==31356==    at 0x40021163: strlen (mac_replace_strmem.c:164)
==31356==    by 0x402FFFBB: writebuf_printf (marshal.c:216)
==31356==    by 0x4030060A: svn_ra_svn_write_number (marshal.c:361)
==31356==    by 0x4030087D: vwrite_tuple (marshal.c:417)
==31356== 
==31356== Conditional jump or move depends on uninitialised value(s)
==31356==    at 0x4002116A: strlen (mac_replace_strmem.c:164)
==31356==    by 0x402FFFBB: writebuf_printf (marshal.c:216)
==31356==    by 0x4030060A: svn_ra_svn_write_number (marshal.c:361)
==31356==    by 0x4030087D: vwrite_tuple (marshal.c:417)
==31356== 
==31356== Syscall param write(buf) contains uninitialised or unaddressable byte(s)
==31356==    at 0x4077A404: __libc_write (in /usr/lib/debug/libc-2.2.5.so)
==31356==    by 0x4045CBF9: apr_socket_send (sendrecv.c:40)
==31356==    by 0x402FFD1D: writebuf_output (marshal.c:156)
==31356==    by 0x402FFE84: writebuf_flush (marshal.c:184)
==31356==    Address 0x41E79104 is 4160 bytes inside a block of size 8244 alloc'd
==31356==    at 0x40028B88: malloc (vg_replace_malloc.c:153)
==31356==    by 0x40465BD6: pool_alloc (apr_pools.c:1277)
==31356==    by 0x402FF8E3: svn_ra_svn_create_conn (marshal.c:50)
==31356==    by 0x402F82BB: ra_svn_open (client.c:586)

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@electricjellyfish.net> writes:

> Philip Martin wrote:
>
>>>   /* Parse the arguments. */
>>>-  SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "l(?r)(?r)bb", &paths,
>>>+  SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "l(?r)(?r)bb?n", &paths,
>>>                                  &start_rev, &end_rev, &changed_paths,
>>>-                                 &strict_node));
>>>+                                 &strict_node, &limit));
>>>+  if (limit == SVN_RA_SVN_UNSPECIFIED_NUMBER)
>> ../svn/subversion/svnserve/serve.c: In function `log_cmd':
>> ../svn/subversion/svnserve/serve.c:897: warning: comparison between signed and unsigned
>> Should limit be apr_uint64_t rather than int?
>>
>
> Interesting.  I don't see that warning here (of course not, Murphy's

gcc -Wsign-compare

> Law dictates that such things should only show up on someone else's
> machine).  I suppose it should, since that's what
> svn_ra_svn_parse_tuple will be expecting.  I will need to pass it into
> svn_repos_get_log3 eventually though, will a simple cast to int be
> correct in that case?

You risk having values greater than INT_MAX converted into negative
numbers (and the svn_repos_get_log3 documentation doesn't say what
happens if limit is negative :)

Of course values greater than INT_MAX won't occur if you are starting
with an int in the first place... which raises a question about
ra_svn_log2: it's probably not valid to pass an int to
svn_ra_svn_write_tuple with an "n" format, as that will attempt to
retrieve 8 bytes from the varargs when only 4 bytes have been passed.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r11155 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_ra_dav libsvn_ra_local libsvn_ra_svn libsvn_repos mod_dav_svn svnserve tests/clients/cmdline/getopt_tests_data

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Philip Martin wrote:

>>   /* Parse the arguments. */
>>-  SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "l(?r)(?r)bb", &paths,
>>+  SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "l(?r)(?r)bb?n", &paths,
>>                                  &start_rev, &end_rev, &changed_paths,
>>-                                 &strict_node));
>>+                                 &strict_node, &limit));
>>+  if (limit == SVN_RA_SVN_UNSPECIFIED_NUMBER)
> 
> 
> ../svn/subversion/svnserve/serve.c: In function `log_cmd':
> ../svn/subversion/svnserve/serve.c:897: warning: comparison between signed and unsigned
> 
> Should limit be apr_uint64_t rather than int?
> 

Interesting.  I don't see that warning here (of course not, Murphy's Law 
dictates that such things should only show up on someone else's 
machine).  I suppose it should, since that's what svn_ra_svn_parse_tuple 
will be expecting.  I will need to pass it into svn_repos_get_log3 
eventually though, will a simple cast to int be correct in that case?

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org