incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject 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/
Date Sat, 31 May 2008 22:22:15 GMT
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

Mime
View raw message