On Mon, Oct 10, 2011 at 7:14 PM, Harshad Prabhu (hprabhu)
<hpra...@cisco.com> wrote:
> Hi Zdenek
>
> Did not get the query correctly. Do you mean input parameter validation ??
>
> Rgds
> Harshad
>
>
Ok, it is my fault.
It is good you're using strtol() function rather than atoi(), because
it does (kind of) validation for you.
However I think you should check whether the last processed character
was '\0' and whether there
was not overflow.
>> + retry = (uint8_t)strtol(optarg, NULL, 0);
~~~
#include <errno.h>
[...]
char *end_ptr = 0;
long retry_long = 0;
errno = 0;
retry_long = strtol(optarg, &end_ptr, 0);
if (errno != 0) { /* overflow occured */ }
if (*end_ptr != '\0' || retry < 0) { /* something like 123abc was
supplied rather than int */ }
/* all good */
retry = (uint8_t)retry_long;
~~~
Checking `retry < 0' might be pointless if retry will be of type of
unsigned long.
Anyway, my point is:
* as of now, it is possible to input number greater than unsigned long
and there is no check
whether user did or did not. Entering such input value will cause
overflow and value of retry_long
is going to be "random"(honestly, I don't know whether values after
overflow can be predicted
or not. But that's beyond the point here)
* it is possible to input something like '123abc' with result being
'123' which is not that bad,
however it should be checked. After all it is invalid input, isn't it?
Also, why not to use strtoul() instead?
~~~
The strtoul() function converts the initial part of the string in nptr
to an unsigned long int value according to the given base, which must
be between 2 and 36 inclusive, or be the special value 0.
~~~
I hope lines above explain better what I had in mind.
As the last thing, please keep the conversation flow. I didn't top
post in my reply, so do not
top-post either, please.
Regards,
Zdenek
> -----Original Message-----
> From: Zdenek Styblik [mailto:zdenek.styb...@gmail.com]
> Sent: Monday, October 10, 2011 10:27 PM
> To: Harshad Prabhu (hprabhu)
> Cc: ipmitool-devel@lists.sourceforge.net
> Subject: Re: [Ipmitool-devel] Regarding Retries/Timeout for LAN/LANPLUS
> interface
>
> On Mon, Oct 10, 2011 at 5:32 PM, Harshad Prabhu (hprabhu)
> <hpra...@cisco.com> wrote:
> [...]
>> + /* Retry and Timeout */
>> + case 'R':
>> + retry = (uint8_t)strtol(optarg, NULL, 0);
>> + break;
>> + case 'N':
>> + timeout = (uint8_t)strtol(optarg, NULL, 0);
>> + break;
>
> Hello,
>
> it seems you're not worried it is possible to overflow `retry' and
> `timeout' which will lead to unexpected behavior.
>
> Regards,
> Zdenek
>
------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure contains a
definitive record of customers, application performance, security
threats, fraudulent activity and more. Splunk takes this data and makes
sense of it. Business sense. IT sense. Common sense.
http://p.sf.net/sfu/splunk-d2dcopy1
_______________________________________________
Ipmitool-devel mailing list
Ipmitool-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
