Skip to content
This repository has been archived by the owner on Jan 5, 2021. It is now read-only.

OCB interfaces should be checked for null in enter() method #1333

Closed
wixiw opened this issue May 1, 2017 · 8 comments · Fixed by #1757
Closed

OCB interfaces should be checked for null in enter() method #1333

wixiw opened this issue May 1, 2017 · 8 comments · Fixed by #1757

Comments

@wixiw
Copy link

wixiw commented May 1, 2017

I'm using an embedded environment without any RTTI available, so NULL pointer are not catched. It's quite painfull to have the board stucked in an HW error each time I forgot to map a new OCB interface.

It would be nice to check the interface mapping in the init() or enter() method.

@terfloth
Copy link
Contributor

OK - checking the operation callbacks makes much sense...

@andreasmuelder
Copy link
Member

the same applies to the Java Code Generator. The operation callbacks should be checked in the enter method. Init could break existing client code.

public void enter() {
if (!initialized) {
throw new IllegalStateException(
"The state machine needs to be initialized first by calling the init() function.");
}

	if(sCInterface.operationCallback == null) {
		throw new IllegalStateException("Operation callback for interface 'scInterface' must be set");
	}
	
	enterSequence_main_region_default();
}

@andreasmuelder andreasmuelder changed the title (C++ interface) OCB interface pointers are never checked OCB interfaces should be checked for null in enter() method Oct 5, 2017
@rherrmannr
Copy link
Contributor

We could implement it like this:

void Default::enter()
{
#ifdef CHECK_UNIMPLEMENTED_VIRTUAL_METHODS
	// Check if methods are implemented
	// if not
	throw std::runtime_error( "Operation callback for interface XY must be set.");
#endif
	/* Default enter sequence for statechart default */
	enseq_main_region_default();
}

and define CHECK_UNIMPLEMENTED_VIRTUAL_METHODS in the StatemachineInterface.h file.
It can be deleted, if some IDE's are not supporting #include

What's your opinion @terfloth ?

@wixiw
Copy link
Author

wixiw commented Oct 9, 2017

This wouldn't answer the need expressed at the issue opening. No RTTI being present you cannot use exceptions. If they where there, I would have had an exception on the NULL pointer.

I may agree that generating a clean runtime error is better than a NULL pointer exception, but it's still using exceptions.

What would fit is a return code to the enter method. It should not break existing code as the function is currently returning a void which is by definition not tested by user code.

@terfloth
Copy link
Contributor

We have to add the checks to ::init() as operation callbacks may be used during variable initialization. Example:

operation getSomeValue(): integer
var x : integer = getSomeValue()

@rherrmannr
Copy link
Contributor

Implemented in Java within the init() function.

@rherrmannr
Copy link
Contributor

As an example I've implemented defines and changed the return type of the init function into the cpp generator.

The first four bits indicates, that an OCB is unimplemented. The four right handed bits indicates, which type of OCB is unimplemented. This can be discussed:
1 -> Internal OCB
2 -> Default Interface OCB
3 -> Any named Interface OCB

The defines in the sc_types.h:

#ifndef OCB_INTERNAL_INIT_ERROR
#define OCB_INTERNAL_INIT_ERROR 0b00010001
#endif	

#ifndef OCB_DEFAULT_INIT_ERROR
#define OCB_DEFAULT_INIT_ERROR 0b00010010
#endif

#ifndef OCB_NAMED_INIT_ERROR
#define OCB_NAMED_INIT_ERROR 0b00010011
#endif

The init function returns three possible errors. They are only generated, if the related interface is used:

sc_integer Default::init()
{
	if (this->iface_OCB == null) return OCB_DEFAULT_INIT_ERROR;
	if (this->ifaceNamed_OCB == null) return OCB_NAMED_INIT_ERROR;
	if (this->ifaceInternalSCI_OCB == null) return OCB_INTERNAL_INIT_ERROR;
      
        //...
	
	return 0;
}

@wixiw
Copy link
Author

wixiw commented Oct 20, 2017

Great solution !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.