You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by Martin Sebor <se...@roguewave.com> on 2008/06/01 00:22:15 UTC
Re: svn commit: r659253 - in /stdcxx/branches/4.2.x: examples/manual/
src/ tests/algorithms/ tests/containers/ tests/localization/ tests/numerics/
tests/regress/ tests/src/ tests/strings/ util/
Eric Lemings wrote:
>
>
[...]
>>> @@ -521,7 +521,7 @@
>>> bad_value (optname, optarg);
>>>
>>> errno = 0;
>>> - defaults->timeout = strtol (optarg, &end, 10);
>>> + defaults->timeout = unsigned (strtol
>> (optarg, &end, 10));
>>
>> I suggest using strtoul() here instead.
>
> strtoul() would still cause a conversion warning, wouldn't it?
Oh. I forgot this was a 64-bit conversion warning and not a signed
vs unsigned one. Sorry.
>
[...]
>>> @@ -581,7 +581,7 @@
>>> errno = 0;
>>> const long code = strtol (optarg, &end, 10);
>>> if ('\0' == *end && !errno)
>>> - exit (code);
>>> + exit (int (code));
>> Seems this code (not necessarily the change) could do with some
>> error checking and reporting...
>
> I noticed the same and not only here but I limited my changes only
> to what the issue calls for.
Fully agreed. Unrelated changes should be made in separate patches.
>
>>> }
>>> }
>>> else if ( sizeof opt_help - 1 == arglen
>>> @@ -595,7 +595,7 @@
>>> && !memcmp (opt_sleep, argv [i],
>> sizeof opt_sleep - 1)) {
>>> /* sleep for the specified number of seconds */
>>> optname = opt_sleep;
>>> - optarg = get_long_val (argv, &i, sizeof
>> opt_sleep - 1);
>>> + optarg = get_long_val (argv, &i, unsigned
>> (sizeof opt_sleep - 1));
>>> if (optarg && *optarg) {
>>> if (!isdigit (*optarg))
>>> bad_value (optname, optarg);
>>> @@ -603,7 +603,7 @@
>>> errno = 0;
>>> const long nsec = strtol (optarg, &end, 10);
>>> if ('\0' == *end && 0 <= nsec && !errno) {
>>> - rw_sleep (nsec);
>>> + rw_sleep (int (nsec));
>> Same here (e.g., passing in a very large number on the command
>> line as a result of a scripting error).
>
> I can start filing minor issues for mo' betta range checking
> on program options when I find such lax usage in the future.
I wouldn't bother with issues for these little things. If it was
me, I'd just go ahead and fix things in a follow-up change after
committing the warning patch.
Martin