On Wed, 25 Sep 2013, Steve Holme wrote:

> So that brings me to what I think is the last warning (hopefully)...

Hah, there's never such a thing as a _last_ warning! =)

> tool_urlglob.c on line 293: The variables being passed into the multiply()
> function are A) an unsigned long pointer and B) an unsigned long
> respectively, however, the function is declared with the second parameter as
> a (signed) long :(
>
> I think this should be also an unsigned long - especially when you consider
> that the first parameter is an in / out parameter so can only return an
> unsigned long as the result - If a negative second parameter was input then
> the output would be negative as well.

Yes, and the function is used to multiple range globbing dimensions to figure
out how many iterations that are needed so they will always be positive
numbers. Unsigned they should be.

> However, fixing that up potentially causes a warning on the "other" usage of
> multiply() on line 114 as its second parameter "size" is declared as an int
> - as per the "content" union in "URLPattern". This variable appears to be
> unsigned in usage although is compared against "ptr_s" which is also
> declared as an int but,, again, appears to be unsigned in usage.

Right. "ptr_s" is the index variable when iterating through a list of strings
and the list is "size" big. Both should be possible to have unsigned.

> After all of that... I guess I have two questions:
>
> 1) Should I fix them up?

Sure! But when doing so you'll get another warning on line 429 when it assigns
'elem' (an int) to the value of that "size" from above... and if you fix
"elem" to also be unsigned, you'll get another warning on line 430 since
checking for >= 0 doesn't make sense for unsigned variables...

Just saying that it will trickle down to a whole slew of edits. Possibly a
shortcut is to just typecast in one of the warning cases.

I am however an eager supporter of removing warnings.

> 2) Is unsigned long the best choice for all of these? Should they be
> unsigned long, unsigned int or size_t?

I picked 'long' just to allow 64bit operations (without any magic involved) on
systems that have big longs. It allows ridiculously large url globbing ranges.
I don't think it is really that necessary so I think that 32bit-long systems
will still be good enough.

If we want to fix 32bit long systems to support ranges
like "[0-2147483648][1-1000]" then we need to switch to something else.
Perhaps then using curl_off_t is good enough since it is "magic" 64bit on most
modern systems including most 32bit systems. *signed* though!

The easier route is to stay with "unsigned long" I think even if it means
support for slightly less insane glob ranges on 32bitlong systems.

-- 
  / daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html