I was recently working on a driver that would fail to work about 50% of the time when the system booted. The driver used to work, and it hadn’t been changed. I started reviewing the change log for the entire BSP, not too many changes to existing files and a few new drivers were added. None of the changes looked like they could affect the problem, or at least not when I was reviewing the code quickly.
After some work, I decided to probe the reset line to the chip that the driver controlled. To my amazement, the chip was in reset. Turns out that the chip doesn’t work so well when it is held in reset.
So after looking at the changes a little closer, I found some code that looks like:
        RetVal = RegQueryValueEx(hKey,TEXT(“Reg Value Name”),NULL,NULL,(LPBYTE)(& Value),&DataSize);
        *CPLDReg |= Value;
If we ignore reality for a moment, that code doesn’t look so bad. It reads a value from the registry and then writes it to a CPLD register.
Okay, back to reality.  There are at least two things that can go very wrong with these two lines of code. They are:
1.       The value might not be in the registry at all. If the value isn’t in the registry, then the call to RegQueryValueEx() fails and the Value variable has some unknown value in it.
2.       The user can modify the registry. That means that the value could be something that could negatively affect the system when written to the CPLD register.
So the code definitely needs some error checking and handling.   The following does that:
        RetVal = RegQueryValueEx(hKey,TEXT(“Reg Value Name”),NULL,NULL,(LPBYTE)(& Value),&DataSize);
        If( RetVal == ERROR_SUCCESS && CheckForValidValue( Value ) )
                *CPLDReg |= Value;
I will leave it up to you to implement CheckForValidValue(), but now we verify that the value was in the registry and that the data was valid before writing it to the CPLD register.
Depending on what this code really does, it might be good to have an else that writes some default value to the CPLD register to initialize it.
Copyright © 2009 – Bruce Eitman
All Rights Reserved