cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: Array Index size_t vs int

From: Marc Hoersken <info_at_marc-hoersken.de>
Date: Sat, 29 Sep 2012 00:16:24 +0200

Hi Daniel,

2012/9/22 Daniel Stenberg <daniel_at_haxx.se>:
> I don't have any problems with using (s)size_t as index in arrays.
>
> I'm talking about that the conversion from those types to int that may exist
> in the code, are sometimes present because of inconsistent APIs in which
> older systems don't use size_t but instead perhaps return ints.

I see that now. For example the functions of internal and external API
related to security, krb5 and rtmp return int values instead of
(s)size_t in most cases.
But in order to avoid the warnings caused by those required
conversions, I started to add warnless functions where necessary.
Attached you will find a series of patches that include those changes.

> But really, I shouldn't speak of such things that genericly. We should talk
> about actual code and discuss specifics instead.

Sure thing. I am sorry for being too general and unresponsive during this week.

Since the attached patches are not really related to the general
issues I was talking about, I would like show you the following
example code from lib/socks.c:

    size_t userlen, pwlen;
[...]
    len = 0;
    socksreq[len++] = 1; /* username/pw subnegotiation version */
    socksreq[len++] = (unsigned char) userlen;
    if(proxy_name && userlen)
      memcpy(socksreq + len, proxy_name, userlen);
    len += (int)userlen;
    socksreq[len++] = (unsigned char) pwlen;
    if(proxy_password && pwlen)
      memcpy(socksreq + len, proxy_password, pwlen);
    len += (int)pwlen;

As you can see, pwlen and userlen are of type size_t and len which is
used to build up the package contents is of type int.
This is just one example where the changes could be made without
breaking external or even internal API.

And as you can see in the following autobuild log, where are probably
many more type conversion problems in the curl source code:
http://curl.haxx.se/dev/log.cgi?id=20120928143826-6174

> I didn't see any actual proposed change so I cannot comment. Changing
> variable types and typecasts will quite possibly change the code. Like
> possibly get different kinds of overflows/wraps or warnings on other
> systems.
>
> I'm just generally not a fan of changing working code for only cosmetic
> reasons.

I understand your concerns and fully agree with you on this. Therefore
I am now proposing patches that only change a variables type if the
chances of breaking something are quite low. In all other cases we
should probably still go the warnless route.

Best regards,
Marc

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

Received on 2012-09-29