Author Topic: SMmemXXX -> Possible bug in libsaio/smblios.c  (Read 2679 times)

0 Members and 1 Guest are viewing this topic.

pouet666

  • Entrant
  • Posts: 5
SMmemXXX -> Possible bug in libsaio/smblios.c
« on: August 04, 2012, 03:05:17 PM »
Hi,

When configuring my new system, and writing the smbios.plist file to fit my hardware, I got stucked with the SMmem** keys.
First of all, I find out reading the source code, that for bank specific options, the syntax "SMxxx_1" is not anymore used. As far as I saw in the source code, we have to write - for example - SMmempart0 for the first module part number, and not SMmempart_1.

Now, back to the main topic. I think there is a small "nested if-then-else" bug in the 'getSMBValueForKey' function. The original one looked like this:

Code: [Select]
if (value)
     if (getIntForKey(key, (int *)&(value->dword), SMBPlist))
          return true;
else
     if (getValueForKey(key, string, &len, SMBPlist))
          return true;

Regarding the indentation, you need brackets, to have something like that

Code: [Select]
if (value)
{
     if (getIntForKey(key, (int *)&(value->dword), SMBPlist))
          return true;
}
else
{
     if (getValueForKey(key, string, &len, SMBPlist))
          return true;
}

I have added a small patch file that can be applied against r2035 in the trunk.


cparm

  • Observer
  • Posts: 12
Re: SMmemXXX -> Possible bug in libsaio/smblios.c
« Reply #1 on: August 05, 2012, 09:14:59 PM »
yep, that's one the reasons why i decided to not force gcc as the default compiler on my branch, the clang static analyzer can help to detect this kind of errors, and trust me when i say that the trunk is full of that kind of "little" errors

anyways i will correct this one , thank

pouet666

  • Entrant
  • Posts: 5
Re: SMmemXXX -> Possible bug in libsaio/smblios.c
« Reply #2 on: August 05, 2012, 09:49:31 PM »
So your branch must be far more stable :)
I will give it a try then...

Jaymonkey

  • Observer
  • Posts: 10
Re: SMmemXXX -> Possible bug in libsaio/smblios.c
« Reply #3 on: November 13, 2013, 01:04:29 AM »
Hi,

When configuring my new system, and writing the smbios.plist file to fit my hardware, I got stucked with the SMmem** keys.
First of all, I find out reading the source code, that for bank specific options, the syntax "SMxxx_1" is not anymore used. As far as I saw in the source code, we have to write - for example - SMmempart0 for the first module part number, and not SMmempart_1.

Now, back to the main topic. I think there is a small "nested if-then-else" bug in the 'getSMBValueForKey' function. The original one looked like this:

Hi pouet666,

I too am battling with the fact that the current build of Chameleon does not seem to parse SMxxxx. However I believe that it can not work because of the nested 'if' statements a little bit further down the code than the bug you spotted. See my bug report here :-

http://forum.voodooprojects.org/index.php/topic,5922.0.html

I Think the issue is the nested if statements at line 470

Code: [Select]
465 if (SMBSetters[idx].keyString)
466 {
467 if (getValueForKey(SMBSetters[idx].keyString, &string, &len, SMBPlist))
468 break;
469 else
470 if (structPtr->orig->type == kSMBTypeMemoryDevice)// MemoryDevice only
471 if (getSMBValueForKey(structPtr->orig, SMBSetters[idx].keyString, &string, NULL))
472 break;
473 }
474 if (SMBSetters[idx].getSMBValue)
475 if (SMBSetters[idx].getSMBValue((returnType *)&string))
476 break;
477 if ((SMBSetters[idx].defaultValue) && *(SMBSetters[idx].defaultValue))
478 {
479 string = *(SMBSetters[idx].defaultValue);
480 break;
481 }
482 string = getSMBStringForField(structPtr->orig, *(uint8_t *)value);
483 break;

It kinda looks like the code is testing for SMMemmoryXXXXXX keys but is returning NULL if true, I didn't write the code but thats what it looks like. I've got the source code now and will have a go at fixing the issue myself as the devs just don't seem to be interested in fixing this bug which has been around for over 18 months now.

Would appreciate anyones input on this thought ????

One thing i find alarming is that you state in your post that the syntax for the SMxxxx keys has changed from SMxxxxx_1 to SMxxxxx0 can you confirm this ? I find it incredible that such a change would be made without letting everyone know. Like I say in my bug report even Chameleon Wizard writes the keys as SMxxxx_1 ... etc

Cheers
Jay
« Last Edit: November 13, 2013, 02:24:15 AM by Jaymonkey »