On Mon, Apr 1, 2013 at 4:10 PM, Zdenek Styblik <zdenek.styb...@gmail.com> wrote:
>> Here's the patched code:
>>
>>         FILE * input_file;
>>         unsigned char data;
>>         unsigned char last_record = 0;
>>         unsigned int multi_offset;
>>         int record_count = 0;
>>
>>         input_file = fopen ( filename, "r");
>>         if ( input_file == NULL ){
>>                 lprintf(LOG_ERR, "File: '%s' is not found", filename);
>>                 return ERROR_STATUS;
>>         }
>>
>>         fseek ( input_file, START_DATA_OFFSET, SEEK_SET );
>>         fread ( &data, 1, 1, input_file );
>>         if ( data <= 0 ){
>
> This has to be changed to: ``if (data == 0)'' or ``if (data < 1)''
> since 'data' is unsigned now or it will yield compiler warning.

Ok, sure...
I guess that this check was saving it from being negative before the
patch as well, which kind of defeats the whole purpose of this patch,
but as long as we're here...

>
>>                 lprintf(LOG_NOTICE, "There is no multi record in the file 
>> %s\n",
>>                 filename);
>
> This LOG_NOTICE -> LOG_ERR

Ok, but I didn't write this :)  I'll change it though.

>
>>                 fclose( input_file );
>>                 return ERROR_STATUS;
>>         }
>>         /*the offset value is in multiple of 8 bytes.*/
>>         multi_offset = data * 8;
>>         if ( verbose == LOG_DEBUG ) {
>
> This looks odd to me. I mean, I've seen code like ``if (verbose)'' or
> ``if (verbose > 2)'' or whatever, but nothing like this. I think this
> is wrong.

Ok, again, this wasn't part of my patch, it was already like this!

I'll change this too though if you want.

thanks!
d

------------------------------------------------------------------------------
Own the Future-Intel&reg; Level Up Game Demo Contest 2013
Rise to greatness in Intel's independent game demo contest.
Compete for recognition, cash, and the chance to get your game 
on Steam. $5K grand prize plus 10 genre and skill prizes. 
Submit your demo by 6/6/13. http://p.sf.net/sfu/intel_levelupd2d
_______________________________________________
Ipmitool-devel mailing list
Ipmitool-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel

Reply via email to