<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta http-equiv="Content-Style-Type" content="text/css" />
<meta name="generator" content="pandoc" />
<title></title>
<style type="text/css">code{white-space: pre;}</style>
</head>
<body>
<p>Hi Rémi,</p>
<p>On 2016-10-28 15:24, Rémi Denis-Courmont wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> + if (strchr("0123456789", *next) == NULL || *end ||</code></pre>
</blockquote>
<pre><code> (((unsigned char)*next) - '0' < 10) is probably much faster/smaller.</code></pre>
</blockquote>
<p>But also less readable, especially if written correctly. I do not imagine that the potential overhead of calling <code>strchr</code> that we have to worry about.</p>
<p>One could write an expression that will be faster (in theory), but I’d view that as micro-optimization (with little gain given the readability issue it would introduce).</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> + port > UINT_MAX || (port == ULONG_MAX && errno == ERANGE))</code></pre>
</blockquote>
<pre><code> I think that this will cause warnings on 32-bits architectures because
port > UINT_MAX is impossible.</code></pre>
</blockquote>
<p>Given the semantics of <code>UINT_MAX</code> and <code>ULONG_MAX</code>, I would assume that any modern compiler would not warn on such usage, and just to verify I created the below test-case.</p>
<pre><code>/tmp% cat bar.c
#include <limits.h>
#include <assert.h>
int main()
{
static_assert( ULONG_MAX == UINT_MAX, "!!" );
unsigned long x = ULONG_MAX;
if( x > UINT_MAX )
return 0;
return 1;
}
/tmp% gcc -Wall -Wextra -pedantic -std=c11 -m32 -c bar.c
/tmp% clang -Wall -Wextra -pedantic -std=c11 -m32 -c bar.c
/tmp%</code></pre>
<p>I also tested with all the 32bit compilers on <em>godbolt.org</em>, with a modified testcase since not all of them has support for C11, and I did not see any such warning.</p>
<p>Though if it turns into an issue I would be happy to prevent such warning.</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> And you cannot rely on errno because it could be the result of an earlier
function call failing.</code></pre>
</blockquote>
<p>I have attached an update patch where the state of <code>errno</code> is set to <code>0</code> prior to the invocation of <code>strtoul</code>, and reset back to whatever state it had before (unless we find invalid data) after.</p>
<p>Thanks for the input!</p>
<p>Best Regards,<br />
Filip</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> + {
+ errno = EINVAL;
ret = -1;
-#endif
+ }
+
+ url->i_port = port;
}
if (url->psz_path != NULL)</code></pre>
</blockquote>
</blockquote>
</body>
</html>