3

Once again, Im not a developer but a wood worker so my questions might be, well, stupid.

I forgot something really important I've to use gcc-3.8 to compile as the original code I'm working with can't compile with newer version. I totaly forgot to talk about that, sorry.

I'm sending data from a tool to an autonomous robot. Robot receive data as unsigned char* I read a lot and it seems that malloc isn't interrupt safe.

As the robot could do bad & dangerous things, I try to make every parts safe (at least as much as I can).

This malloc happens after an interrupt raised by data received. This code segment is now giving me an hard time to make it safe, also my syntax is probably bad..

char* _Xp = (char*) malloc(strlen((char*)_X)*sizeof(char));
strcpy(_Xp, (char*)_X);

1) is malloc really not interrupt safe? information I found are from 2004.

2) is there a more efficient way to initialize the buffer?

3) why are unsigned char "bad"? (read something about that).

4) last question is strcpy also not interrupt safe? sources I read differ on that point.

===== answering some questions:

Robot doesn't have operating system, target is an STR911FFM44 at 25Mhz (if it helps)

Input arrays are all nul-terminated.

Code is not in the interruption handler but in the infinite loop and processed only if the IrHandler as set the flag for it.

I don't know the rate of the data stream to "hard code" the safety. but interruption should be in [500ms to 1500ms].

Lundin
  • 195,001
  • 40
  • 254
  • 396
A.albin
  • 274
  • 2
  • 15
  • I don't suppose you know whether or not `_X` is *nul-terminated*, do you? Also, there is NO need to cast the return of `malloc`, it is unnecessary. See: [**Do I cast the result of malloc?**](http://stackoverflow.com/q/605845/995714) for thorough explanation. – David C. Rankin Apr 05 '17 at 05:33
  • You can use `strdup` (if you actually have a nul-terminated string). Not safer than `malloc`, though. – Ry- Apr 05 '17 at 05:33
  • its null terminated, I make that sure by reserving the last byte and hard coding it. – A.albin Apr 05 '17 at 05:34
  • Are you adding `+1` in your `malloc` call to provide space for it in `_Xp` before the `strcpy`? – David C. Rankin Apr 05 '17 at 05:35
  • 3
    (1) Your information is accurate — `malloc()` is one of the least interrupt-safe routines around. (2) Don't do the memory allocation in the interrupt handler. In your main loop, spot that an interrupt occurred so there is data to read and then allocate the buffer and read the data. (3) There's no particular reason to avoid `unsigned char` if that's the type you need to work with. The only reason for caution is that the library functions almost all take `char *` and not `unsigned char *`, so using `unsigned char` can make things verbose. – Jonathan Leffler Apr 05 '17 at 05:35
  • 2
    You have two sane choices, but they both come down to doing less work in the interrupt handler. First, you can have a buffer already set up for the interrupt handler and have separate code that fills up the supply of buffers for the interrupt handler. Second, you can have the interrupt handler do nothing but tell other, non-interrupt code to handle whatever it is that needs to be handled. Essentially, do as little as possible in the interrupt handler, and you definitely don't need to allocate memory in it since that can be done any time. – David Schwartz Apr 05 '17 at 05:37
  • @DavidC.Rankin _X is already one byte larger before hand, should I again add another byte? – A.albin Apr 05 '17 at 05:45
  • What sort of programming environment is the robot using? Does it have an operating system? Interrupt handlers should avoid doing heavy lifting; their interaction with user code is usually mediated by the OS, which allows `malloc` to be safe. – Potatoswatter Apr 05 '17 at 05:45
  • @DavidSchwartz the code is in a function that start only if I got an interruption and the received_data flag as been set to true, is it sufficient to make it safe? – A.albin Apr 05 '17 at 05:46
  • @A.albin - no. It is simply an accounting problem. If you have already accounted for the space required for the *nul-byte* there is no need to tack an extra on the end. It was just completely unclear from what you posted. The answer lies in the interrupt sensitivity described by the other commenters. – David C. Rankin Apr 05 '17 at 05:52
  • 3
    There are no stupid questions. You being a wood worker has nothing to do with the quality of your post, mainly because you did way more research than the novice "professional" programmer does on average today. – StoryTeller - Unslander Monica Apr 05 '17 at 05:52
  • 1
    You can qualify that slightly by saying *"the only stupid questions are the ones that go unasked"* `:)` and if @A.albin writes the first bit of code to auto-square my Beismeir Fence System, I'll be the first to use it... – David C. Rankin Apr 05 '17 at 05:53
  • @DavidC.Rankin - hehe, but I don't think going unasked attests to the inherent quality of the question :) – StoryTeller - Unslander Monica Apr 05 '17 at 05:54
  • Granted......... – David C. Rankin Apr 05 '17 at 05:55
  • Most likely, [using malloc doesn't even make any sense](http://electronics.stackexchange.com/a/171581/6102) in this application. – Lundin Apr 05 '17 at 07:34
  • Not only is it not safe, it is non-deterministic in terms if processing time and success /failure. strlen and strcpy are also poor choice for ISR; their execution time depends on string length, making the ISR less deterministic. – Clifford Apr 05 '17 at 12:23
  • 1) On typical MCU environments, `malloc` & friends should be avoided like hell. 2) They definitively must not be used in an interrupt handler. 3) Names starting with underscore at file-level are reserved for the implementation for all name-spaces. IOW: you must not define them in your code. – too honest for this site Apr 05 '17 at 17:20

2 Answers2

4

1) is malloc really not interrupt safe?

malloc accesses and modifys a global resource, the common memory pool of you running program. If the access happens from two unsynchronized places, such as your normal program flow and the ISR1, then it can mess up the pool. If your ISR doesn't call malloc itself it won't be a problem.

If it does, you'd need to put in place a system for preventing such reentry into malloc. For instance, wrap the call to malloc in a function that turns interrupt handling off, and then on again.

2) is there a more efficient way to initialize the buffer?

If you need a buffer with allocated storage duration (i.e. that you decide when its lifetime ends, and not the scope it's allocated in) then there isn't really a standard C alternative. By the way, sizeof(char) is always 1, so no need to specify it. And since C allows implicit conversion of pointer types from void*, the call can at least be prettified a bit2:

char* _Xp = malloc(strlen((char*)_X));

3) why are unsigned char "bad"?

They aren't bad. In fact when you need to know exactly whether or not the character type is signed or not, you have to use signed char or unsigned char. The regular char can be signed on one platform and unsigned on another.


1 Interrupt Service Routine.
2 C has a notion of reserved identifiers. In particular, any identifier the starts with an underscore followed by an uppercase letter is always reserved. So renaming your variables may help with portability.

Community
  • 1
  • 1
StoryTeller - Unslander Monica
  • 165,132
  • 21
  • 377
  • 458
  • OK, understood will update my variable names, even if I will be the only one using it. – A.albin Apr 05 '17 at 06:09
  • @A.albin - It's not to protect other people. The identifiers are reserved because sometimes the language updates with new keywords (for instance in 1999 `_Bool` was added and in 2011 `_Noreturn`). It'd be a shame to have code break when a newer compiler is used. – StoryTeller - Unslander Monica Apr 05 '17 at 06:11
  • I have to compile with gcc 3.8 (its an old hardware) I think I should have said it before.... – A.albin Apr 05 '17 at 06:25
  • @A.albin Even if you are using and old compiler, the underscore means the code might collide with identifiers used by the compiler. Most often this is not an issue - I've maintained lots of code bases using such naming conventions and never encountered any bugs caused by it. But in theory, you could get unlucky and use the same identifier as the compiler. – Lundin Apr 05 '17 at 08:18
1

First of all, you say that you are using a bare metal microcontroller, so malloc never makes sense. It is not a PC - you don't share your RAM with anyone else. So all the dangers and disadvantages of malloc don't even enter the discussion, since malloc makes no sense for you to use at all.

1) is malloc really not interrupt safe? information I found are from 2004.
4) last question is strcpy also not interrupt safe? sources I read differ on that point.

No function using shared resources between an ISR and the main application is interrupt safe. You should avoid calling library functions from an ISR, they should be kept minimal.

All data shared between an ISR and the caller has to be treated with care. You must ensure atomic access of individual objects. You must declare such variables as volatile to prevent optimizer bugs. You might have to use semaphores or other synchronization means. This applies to all such data, whether you change it by yourself or through a library function.

Failing to do all of the above will lead to very mysterious and subtle bugs, causing data corruption, race conditions or code that is never executed. Overall, interrupts are always hard to work with because of all this extra complexity. Only use them when your real-time requirements give you no other options.

2) is there a more efficient way to initialize the buffer?

Yes, use an array. static char _Xp [LARGE_ENOUGH_FOR_WORST_CASE]; Usually it is a good idea to keep such buffers in the .data segment rather than on the stack, hence the static keyword.

3) why are unsigned char "bad"? (read something about that).

There is nothing bad with them as such. The different char types are problematic though, because in theory they could have other sizes than 8 bits. Worse, char without signed/unsigned has implementation-defined signedness, meaning it might be signed or unsigned depending on compiler. Meaning that you should never use the char type for storing anything else but text strings.

If you need a variable type to hold bytes of data, always use uint8_t from stdint.h.

As the robot could do bad & dangerous things, I try to make every parts safe (at least as much as I can).

Writing safe software for embedded systems is a highly qualified task. I wouldn't recommend anyone with less than 5 years of experience working full-time with embedded firmware programming to even consider it, unless there's at least one hardened C veteran who is part of your team and all code passes through peer review and static analysis.

It sounds like you would benefit a lot from reading through the MISRA-C:2012 coding guidelines. It is a safe subset of the C language, intended to be used in safety-critical applications, or any form of application where bugs are bad. Unfortunately the MISRA-C document is not free, but it is becoming an industry standard.

Community
  • 1
  • 1
Lundin
  • 195,001
  • 40
  • 254
  • 396