3

I work with C/CPP on Embedded systems, and in my code I have some sections that is run from some memory that is "invalid" and MUST NOT be called until some condition is met.

for simplicity:
Lets say foo() is such function and is invalid while g_isMemoryValid == false

except for foo() there are couple of houndred other functions in the invalid memory section with several calls each from all the modules.

my problem is to find all instances where i enter the invalid section and verify i didnt miss any call!

How can i ensure that all branches where foo() is run are meet g_isMemoryValid == true

Solution Should output:
The line which jumps to the code(or instruction address), but even if it will emit the module name only,
Or Hell... [OK, NOT_OK] will do :)

Note: Even solutions that will solve only subset of the scenarios will do!

Notice that there many scenarios in addition to simple if(g_isMemoryValid) wrapper like call chaining the calling function in itself not called while condition is not met or some different syntax of the branch

if (g_isMemoryValid) 
    foo(); // should pass

if (!g_isMemoryValid)
    return; 
foo(); // should pass too
Tomer W
  • 3,395
  • 2
  • 29
  • 44
  • Wrap them in a mutex which is unlocked when `g_isMemoryValid` becomes true? – Shark Mar 01 '16 at 14:32
  • Do you have access to the source code of `foo()` so that you can modify the function itself? Alternatively, can you implement a wrapper function for `foo()`? – Andreas Fester Mar 01 '16 at 14:32
  • 4
    `#define CHECKED_FOO() if (g_isMemoryValid == true) foo();` – LPs Mar 01 '16 at 14:33
  • @LPs has a very elegant solution indeed. +1 for that. – Shark Mar 01 '16 at 14:35
  • 1
    @LPs You should post that as an answer. – Magisch Mar 01 '16 at 14:35
  • 1
    I don't get your question, you know how to check for the condition and how to call the function only if the condition is true, so that's not a problem. Are you asking for the most "elegant" way of doing it? Then "elegant" according to who? That's a very opinion based question. Also, using macros as suggested by some might seem tempting, but IMHO it's not an "elegant" solution, and is very opaque about what's happening. – Some programmer dude Mar 01 '16 at 14:39
  • `CPP` is the C preprocessor which is part of the language C. If you mean C/C++, there is no such language. Only the **different** languages C and C++. In any language explicitly comparing a boolean value with a boolean constant is bad style. Just use `if ( g_isMemoryValid )` or its negation. – too honest for this site Mar 01 '16 at 14:41
  • @Magisch Thanks, but It was the first/faster solution. Maybe OP should try to understand how to avoid using a global flag `g_isMemoryValid` to do what he needs and, e.g., to develop a module with an interface that valid or not a call to a specific function. – LPs Mar 01 '16 at 14:48
  • Is your code multithreaded or do you have any kind of interrupt service routines that can affect the value? – Juan Leni Mar 01 '16 at 14:48
  • i explained myself wrong... ill edit the question, i want to find where should i add a "condition check" – Tomer W Mar 01 '16 at 14:49
  • @Olaf For my personal knowledge: what is the problem using `stdbool.h` and write `if ( g_isMemoryValid == true )` or `if ( g_isMemoryValid == false)` ? – LPs Mar 01 '16 at 14:49
  • Similar to @LPs answer, you could define a macro that expands based on the parameters given it (ie. `#define CHECK_COND(var,fxn) if(var) fxn;`). Or you could wrap the function you want to call in another function that does the if check for you to ensure the `foo()` function is never called when your check condition is invalid. – callyalater Mar 01 '16 at 14:50
  • Is there a reason I am missing why a simple *inline* function wrapper won't suffice? – Galik Mar 01 '16 at 14:52
  • @Galik Same as using macro, with the [difference between macro and inline functions](http://stackoverflow.com/a/13375389/3436922) – LPs Mar 01 '16 at 14:55
  • @LPs: It requires another thought interpreting the comparison operator. Something like `if (x_valid )` is more like th natural language we learned much earlier. (disclaimer: I just talk about western languages - no offence, I just don't know other languages well enough). It is not about using or not using `stdbool.h`. – too honest for this site Mar 01 '16 at 14:58
  • @LPs: Another common reason not to compare against `true` or `false` literals is that in C++, there can be automatic conversion from pointers and different types to boolean types. This allows for conditional checking on multiple types like `T* t = nullptr; if(t){...}` where a conversion will take place between `nullptr_t` and `boolean`. – callyalater Mar 01 '16 at 15:05
  • also in some unique architectures, true is not 1 and false is not nessecarilly 0. but i used them just for clarity. – Tomer W Mar 01 '16 at 15:07
  • Most decent `IDEs` will provide a search function otherwise tools like `grep` can be useful here. – Galik Mar 01 '16 at 15:08
  • @Galik this is more in the direction, but the syntax is too complex to know if the call is protected or not... i remind you have houndred of functions like Foo, and several calls each. – Tomer W Mar 01 '16 at 15:14
  • Have you the control of the process that copy function to memory? If yes, could be this thread/function/module or whatever the root of your application that kick-off all other modules/threads. You are talking about embedded systems, maybe your bootloader can pre-load all you need. – LPs Mar 01 '16 at 15:47

2 Answers2

3

One solution that, in my opinion at least, is more elegant than a macro would be to use a function pointer:

#include <stdio.h>

// The real foo()
void foo()
{
    puts("OK");
}

// Handle 'invalid' memory (error message?)
void mem_invalid()
{
    puts("Not OK");
}

typedef void (*foo_t)();

foo_t foo_ptr = &mem_invalid;

void main(void)
{
    // Memory is 'invalid', calls error routine
    foo_ptr();
    ...

    // Memory became good somehow
    foo_ptr = &foo;
    ...

    // Call now succeeds
    foo_ptr();
}

This uses the function pointer in lieu of the flag.

Depending on your embedded system there may be some overhead associated with the indirect function call but unless it is in a critical area like an inner loop it shouldn't be an issue. The error handler doesn't even need to do anything - or it can be as complicated as you like.

Trevor
  • 311
  • 2
  • 6
  • Some safety-oriented coding style guides on embedded systems disallow function pointers. – too honest for this site Mar 01 '16 at 15:48
  • @Olaf, what coding style are you referring to? Misra C Rule 104, which has since been rescinded, used to say 'Non-constant pointers to functions shall not be used' but my understanding is that this was intended to mean 'pointers to functions shall not be calculated at runtime.' The risk is from errors in calculating the address and this won't occur here because the address is known at compile time and will be encoded as a literal. In this case it is no less safe than a direct call. – Trevor Mar 01 '16 at 16:29
  • It si not just because of this, but also because the pointer can get corrupted by data accesses. I agree such rules are questionable, but that does not change the fact they exist. I will not defend them, though. – too honest for this site Mar 01 '16 at 16:32
  • @Olaf, if you mean that the function pointer can be overwritten - perhaps by using an out of range array subscript or an invalid pointer, yes that is a risk, but if your code contains bugs like that then the function pointer is likely the least of your problems (the corrupted memory location could just as easily contain a data pointer or array subscript variable for instance, or a loop counter causing an infinite loop, or...). – Trevor Mar 02 '16 at 01:46
  • Interestingly J Holzmann from NASA/JPL also did a paper 'The Power of 10: Rules for Developing Safety-Critical Code' where rule 9 states to not use function pointers. But if you read his paper, the rationale is that function pointers hide possible recursion from static checking tools, rather than any concerns about memory corruption. Recursion obviously has a risk of resource exhaustion (e.g. stack space), which will be a lot harder to check for than memory corruption. – Trevor Mar 02 '16 at 01:52
  • The "no function pointer" is some religious dogma with a weak rationale. The mentioned NASA paper doesn't really make much sense for safety-critical systems, [as discussed here](http://stackoverflow.com/a/34950753/584518). – Lundin Mar 02 '16 at 09:30
  • That being said, you can easily add defensive programming to the posted program. Create an array of pointers to all valid functions that the function pointer is allowed to call, and before calling your function pointer, iterate through the array and ensure that the function actually exists. Or if you will, a simple `assert(foo_ptr == foo || foo_ptr == mem_invalid)`. – Lundin Mar 02 '16 at 09:39
2

If you cannot modify the way foo is called, you have to modify foo itself:

  • rename the foo function to inner_foo
  • write yourself a foo that does all the checks, prints all the messages, and, wherever it's all OK, call itself inner_foo
  • recompile the module containing the old foo and your new foo and relink all the modules where foo is called.

now all branches will call your own new foo function.

Emilio Garavaglia
  • 20,229
  • 2
  • 46
  • 63
  • This is, by far, the cleanest and most straight-forward way to do this. For what its worth, since the post is tagged `C++` too... the wrapper could `throw` if the memory is invalid. – Brian McFarland Mar 01 '16 at 15:20