Fault Tolerant Programming in RealTime

[DieselPower]

Is this OK:

TCPWriteMessageString("Jumped Mode"); 

Or should I say:

TCPWriteMessageString("Jumped Mode\0"); 

This is just one of the potential issues I'm looking at to try to solve this problem.

[rolfk]

---

@DieselPower wrote:

Is this OK:

TCPWriteMessageString("Jumped Mode"); 

Or should I say:

TCPWriteMessageString("Jumped Mode\0"); 

This is just one of the potential issues I'm looking at to try to solve this problem.

---

Any standard complying C compiler is supposed to expand quoted string constants with an automatic NULL termination character since this is THE method in C to delimit strings. So your second statement is not necessary, effectively it will make the actual string constant having two NULL characters at the end. TCPWriteMessageString() using internally most likely standard C runtime library functions like strlen() to determine the string length won't see the second NULL character anyways as it will terminate on the first already.

However C programming not having exception handling like you are used in C++, requires you to rigorously validate any pointer evaluation before trying to dereference it. So the original code in this thread should look more like this:

#define checkPointer(pointer) if (!pointer) return ERROR_INVALID_POINTER

token = strtok(buffer, "\t");
checkPointer(token);																
if (strstr(token, "ENGINE CONTROL"))
{
    token = strtok(NULL, "\t");
    checkPointer(token);
    CLControls[0]->FinalSetpoint = atoi(token);  			
    token = strtok(NULL, "\t");
    checkPointer(token);
    CLControls[1]->FinalSetpoint = atoi(token);  
    token = strtok(NULL, "\t");
    checkPointer(token);
    CLControls[0]->RampLength = CLControls[1]->RampLength = atoi(token);
    CLControls[0]->ThisSetpoint = EngineSpeed; 					
    CLControls[0]->LastSetpoint = EngineSpeed; //CLControls[1]->LastSetpoint = CLControls[1]->ThisSetpoint;
    CLControls[1]->ThisSetpoint = DL5.Torque; 					 
    CLControls[1]->cumError = 0;
    CLControls[0]->Active = CLControls[1]->Active = 1;
}

In addition you may also need to verify that CLControls is a valid pointer and contains the number of elements you want to dereference later on.

And one more remark: What do you expect strtok() to do with a NULL value as first parameter??????? The standard library documentation for this function does not state that passing a NULL value is a valid option for this function so there is no requirement for it to check for that possibility. It only states that the function can return a NULL value if the token pattern could not be found in the string, which is not the same. Neither is atoi() required to test for a valid input string pointer before trying to access it.

The error happening inside the C runtime library rather than in your own C code is most likely the reason that the debugger does not break into the function as there is no source code for the C runtime library to break into. 

[RobertoBozzolo]

While I second all notes expressed by Rolf, I disagree with him on strtok: as you can see in the help for the function, NULL is the keyword required by the command "to continue searching where the last successful function call ended"

[rolfk]

I stand corrected about the NULL value for the first parameter. I did not refer to the online LabWindows CVI documentation but rather to the documentation found here and it does not describe this behaviour (but shows it in the example further down).

In terms of multithreading safety, functions which maintain an internal state between calls are a TERRIBLE thing to do, so I didn't quite expect this behaviour, but the C runtime library is indeed riddled with such bad functions. Still the documentation clearly states that the function can return a NULL pointer and passing such a pointer unchecked to another C runtime function is never safe.

[DieselPower]

I appreciate the support this morning guys.  I will implement this suggestion today and also look for other locations in the program where such a flaw exists. 

I still need to find that pesky missing null terminator, however. 

[DieselPower]

Please forgive my ignorance on the macro.

I'm going to leave what I wrote below for reference, but it looks like the macro will return out of the function altogether.  Is that correct?

***************************************************************************************************************

I have defined the macro as such.  I added a semicolon whereas your input did not include it.

#define checkPointer(pointer) if (!pointer) return ERROR_INVALID_POINTER;

I have also added a constant because I don't know where ERROR_INVALID_POINTER is declared.

#define ERROR_INVALID_POINTER -1 

I have tried to use it in this manner, but I get an error: expected expression.  If I remove the if statement then it compiles, but I don;t know exactly what I should do to handle checkPointer returning ERROR_INVALID_POINTER.  Your input does not handle the potential error.  It only checks for it.

if (strstr(token, "ENGINE CONTROL"))
{
    token = strtok(NULL, "\t");
    if (checkPointer(token) != -1)
    {
        CLControls[0]->FinalSetpoint = atoi(token);  			
        token = strtok(NULL, "\t");
        if (checkPointer(token) != -1)
        {							
            CLControls[1]->FinalSetpoint = atoi(token);  
            token = strtok(NULL, "\t");
            if (checkPointer(token) != -1)
            {								
                CLControls[0]->RampLength = CLControls[1]->RampLength = atoi(token);
                CLControls[0]->ThisSetpoint = EngineSpeed; 					
                CLControls[0]->LastSetpoint = EngineSpeed; //CLControls[1]->LastSetpoint = CLControls[1]->ThisSetpoint;
                CLControls[1]->ThisSetpoint = DL5.Torque; 					
                CLControls[1]->cumError = 0;
                CLControls[0]->Active = CLControls[1]->Active = 1;
            }
        }
        else
        {
            DebugPrintf("Error processing ENGINE CONTROL message!\r\n");
        }							
    }
    else
    {
        DebugPrintf("Error processing ENGINE CONTROL message!\r\n");
    }
}
else
{
DebugPrintf("Error processing ENGINE CONTROL message!\r\n");
}

[rolfk]

C programming can be hard and macro magic even more!

Do not add a semicolon to the define as that will have unintended effects when you use the macro as an expression. Also the macro as I showed it was just a suggestion about how to do it, not a canonical implementation. The ERROR_INVALID_POINTER is simply a place holder for whatever error value you want to return in your function for such situations. It could be also MyLibraryErrorInvalidParameterAndSomethingElse.

Basically the checkPointer() macro as I proposed it was meant to be used after you assign a value to a pointer variable. As it looks like a function and works more or less like a function you will add the semicolon after the invocation of the macro, so it is not necessary to have a semicolon in the macro itself.

If you want to check the pointer in an if statement you will of course not need the macro itself but instead just write:

if (token != NULL) // or if (token)  which is the same
{

}

The use of the macro however as I showed it in the code will avoid that you have to write something like this:

token = strtok(buffer, "\t");
if (token)														
{
    if (strstr(token, "ENGINE CONTROL"))
    {
        token = strtok(NULL, "\t");
        if (token)
        {
            CLControls[0]->FinalSetpoint = atoi(token);  			
            token = strtok(NULL, "\t");
            if (token)
            {
                CLControls[1]->FinalSetpoint = atoi(token);  
                token = strtok(NULL, "\t");
                if (token)
                {
                    CLControls[0]->RampLength = CLControls[1]->RampLength = atoi(token);
                    CLControls[0]->ThisSetpoint = EngineSpeed; 					
                    CLControls[0]->LastSetpoint = EngineSpeed; //CLControls[1]->LastSetpoint = CLControls[1]->ThisSetpoint;
                    CLControls[1]->ThisSetpoint = DL5.Torque; 					 
                    CLControls[1]->cumError = 0;
                    CLControls[0]->Active = CLControls[1]->Active = 1;
                 }
             }
        }
    }
}

This code is functionally equivalent to the code in my earlier post (unless you have to perform some cleanup code before exciting the function) but looks a lot less clean and maintainable.

[DieselPower]

Rolf, this is outstanding support.  I've never had such fast responses.

I think I understand the macro deal now.  I have looked up macro arguments to understand the process.

Now it looks like the safest method to run this code would be to apply the new values to local variables until all of them are processed at which time the new values would be applied to the actual variables.  This would eliminate the possibility of setting one value and then failing on the next and bouncing out. In the latter case engine control would move to a different speed for instance, while keeping torque the same.  Is this correct?

Last question for now:

This code is inside of a callback function defined as follows:

static int CVICALLBACK TCPCallback (unsigned int handle, int xType, int errCode, void *callbackData) 

There is no line of code that processes the return value because it is fired when new TCP data is available.  At the end of my function I reset the buffer[0] to '\0'

I should add that to my macro as follows, correct?

#define checkPointer(pointer) if (!pointer) buffer[0] = '\0'; return ERROR_INVALID_POINTER 	
  

[rolfk]

Not quite! You want to define your macro rather like this because otherwise the return statement will be always hit since it is not part of the conditional block for the if statement.

#define checkPointer(pointer) if (!pointer) {buffer[0] = '\0'; return ERROR_INVALID_POINTER;} 

Of course I would probably add the buffer value as a second macro parameter to it as otherwise the macro is strictly limited to functions where a buffer variable declaration is visible. 

[DieselPower]

Is there any way to track a thread by ID?

FATAL RUN-TIME ERROR:   Unknown source position, thread id 0x000000EA:   Null pointer argument to library function.