On Fri, Jan 31, 2014 at 3:08 PM, Zdenek Styblik
<zdenek.styb...@gmail.com> wrote:
> On Fri, Jan 31, 2014 at 2:29 PM, Petter Reinholdtsen <p...@hungry.com> wrote:
>> While we wait to conclude if we should move to git or not, making me
>> unsure if I should commit to CVS or git, here is a small patch to fix
>> a bug discovered by Coverity.
>>
>> The problem is that the value of rv, extracted from
>> serial_read_line(), is checked if it is negative. The unsigned long
>> value can never be negative, causing errors reported from
>> serial_read_line() to be ignored.
>>
>> I propose to fix it by making the temp result variable local to the
>> scope where it is used, and having the same type as is returned from
>> the function used to give it a value. This patch should fix CID
>> 1149026.
>>
>> Index: src/plugins/serial/serial_terminal.c
>> ===================================================================
>> RCS file: /cvsroot/ipmitool/ipmitool/src/plugins/serial/serial_terminal.c,v
>> retrieving revision 1.4
>> diff -u -3 -p -u -r1.4 serial_terminal.c
>> --- src/plugins/serial/serial_terminal.c 31 Jan 2014 06:13:58 -0000
>> 1.4
>> +++ src/plugins/serial/serial_terminal.c 31 Jan 2014 13:25:09 -0000
>> @@ -362,12 +362,12 @@ recv_response(struct ipmi_intf * intf, u
>> {
>> char hex_rs[IPMI_SERIAL_MAX_RESPONSE * 3];
>> int i, j, resp_len = 0;
>> - unsigned long rv;
>> char *p, *pp;
>> char ch, str_hex[3];
>>
>> p = hex_rs;
>> while (1) {
>> + int rv;
>> if ((rv = serial_read_line(intf, p, sizeof(hex_rs) -
>> resp_len)) < 0) {
>> /* error */
>> return -1;
>> @@ -400,7 +400,7 @@ recv_response(struct ipmi_intf * intf, u
>> sleep(1);
>> serial_flush(intf);
>> errno = 0;
>> - rv = strtoul(p + 4, &p, 16);
>> + unsigned long rv = strtoul(p + 4, &p, 16);
>> if ((rv && rv < 0x100 && *p == '\0')
>> || (rv == 0 && !errno)) {
>> /* The message didn't get it through. The upper
>>
>
> Either I'm missing something, or your patch is wrong. Either way, I
> really don't like your approach to this problem. I'm sorry, but this
> is fugly.
>
> Z.
Please, just declare new variable, be it rv_readline, with type of int
and don't do things which cause a sudden need to suffocate myself with
a big black whooper ;)
Z.
>
>
>> As soon as we conclude about cvs vs. git, I'll commit into the winning
>> repository. :)
>>
>> --
>> Happy hacking
>> Petter Reinholdtsen
>>
>> ------------------------------------------------------------------------------
>> WatchGuard Dimension instantly turns raw network data into actionable
>> security intelligence. It gives you real-time visual feedback on key
>> security issues and trends. Skip the complicated setup - simply import
>> a virtual appliance and go from zero to informed in seconds.
>> http://pubads.g.doubleclick.net/gampad/clk?id=123612991&iu=/4140/ostg.clktrk
>> _______________________________________________
>> Ipmitool-devel mailing list
>> Ipmitool-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
------------------------------------------------------------------------------
WatchGuard Dimension instantly turns raw network data into actionable
security intelligence. It gives you real-time visual feedback on key
security issues and trends. Skip the complicated setup - simply import
a virtual appliance and go from zero to informed in seconds.
http://pubads.g.doubleclick.net/gampad/clk?id=123612991&iu=/4140/ostg.clktrk
_______________________________________________
Ipmitool-devel mailing list
Ipmitool-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
