2012/4/17, Daniel Stenberg wrote:

>> Ok. Although url.c:1485 provides a couple of 'features' that I'm not sure
>> if they are fully intended.
>
> Oh right...
> [...]
> That's indeed unintentional. That check really needs to be enhanced. Don't
> we also have something similar elsewhere? Yes, only CURLAUTH_ONLY is
> not really a valid bitmask.

I'll leave those 'adjustments' for later on, given that these are just
internal stuff we should be able to adapt as needed without disturbing
external API.

I really want we focus first on the CURLAUTH_* constants and
appreciate any comments on this...

1) Current definition of CURLAUTH_ONLY as (1<<31) is not a valid 32bit
signed int, its usage introduces compiler dependent behavior. So let's
try to fix it without breaking ABI.

2) First appealling idea is to define it as (1U<<31) making all
CURLAUTH_* bitmasks unsigned int. Nice but not enough yet.

3) CURLAUTH_* bitmasks are or'ed together and passed to
curl_easy_setopt() as third argument when using CURLOPT_HTTPAUTH or
CURLOPT_PROXYAUTH options. But notice here that the third argument
curl_easy_setopt() expects a 'long' data type. This results in an
implicit conversion from 'unsigned int' to 'long' or an explicit one
when third argument has been casted to 'long'.

On systems where sizeof(long) is greater than sizeof(int), and current
(1<<31) definition is being used the conversion is taking place doing
sign-extension. IOW the CURLAUTH_ONLY bit passed from the external API
is seen internally as a bit indicating negative number. Reason for
current clever avoiding of internal comparison with CURLAUTH_ONLY.

We can avoid bitmask bits moving around when passed through
curl_easy_setopt() making CURLAUTH_* masks of 'long' data type. This
should also allow CURLAUTH_ONLY to be used internally.

And given that on systems with a 32bit long, (1L<<31) is not yet a
valid signed long...

3) I believe that we should change CURLAUTH_* bitmasks to become
'unsigned long' typed.

Now we have two options for the definition..

The nice looking one which should work nearly with any compiler:

#define CURLAUTH_ONLY (1UL<<31)

Or a not so nice looking one which should work with all compilers:

#define CURLAUTH_ONLY (((unsigned long)1)<<31)

Basically thats my reasoning.

-- 
-=[Yang]=-
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html