Passing array pointer correctly?

[katzohki]

Hi all,

I have an application I'm working on and I'm having some trouble moving the test code from my main.c and adjusting it to take the correct argument. To start with, I'm storing a message in the eeprom and then copying it into RAM at runtime:

Code: [Select]

//Includes
#include <stdio.h>
#include <string.h>

...

//Global Variables
eeprom char MSG[] = "DE 1234    ";    //stored in eeprom
char message[] = ' ';

...

(in main)
int len =0;
len = sizeof(MSG);
memcpy(message,MSG,len);
This works, but I'm happy to hear if there's a better way.

The function I'm trying to move is this:

Code: [Select]

void morse_test(void){
    char *cp;
    cp = message;
    while(*cp)
        sendM(*cp++);
}
In my external C file it looks like this:

Code: [Select]

void sendMstr(char A){
    char *cp;
    cp = A;   //MSG
    while(*cp)
        sendM(*cp++);
}
So you should be able to do sendMstr(message); in main.c but that does not work. (specifically, it sends DE and then restarts). Any help is appreciated! Thanks!

[andersm]

This works, but I'm happy to hear if there's a better way.

It doesn't, because "message" does not have enough space to store "MSG". You're writing over something in memory, you just don't know it.

In my external C file it looks like this:

Code: [Select]

void sendMstr(char A){
    char *cp;
    cp = A;   //MSG
    while(*cp)
        sendM(*cp++);
}

"A" is a single char parameter, passed by value. You can take its address, but you can't iterate on that address because you will be accessing random memory. What you want to do is pass a pointer to the string into the function:

Code: [Select]

void sendMstr(char *A){
    while(*A)
        sendM(*A++);
}


[IanB]

eeprom char MSG[] = "DE 1234    ";    //stored in eeprom

This is OK.

char message[] = ' ';

This is not OK. You have allocated an array of size 1.

(in main)
int len =0;
len = sizeof(MSG);
memcpy(message,MSG,len);

Here you are trying to copy a long message into an array of size 1. The array will overflow, you will clobber memory, and nothing good will come of this.

[katzohki]

Code: [Select]

void sendMstr(char *A){
    while(*A)
        sendM(*A++);
}


Thanks, that helped. I thought I needed to be using a pointer, but I guess I was missing something.

As for the memory issue with the message array, I can see what you mean about the memory space being an issue since addresses are being manipulated. What do I do besides reserve a really huge array in RAM?

[Howardlong]

It only has to be as large as the source string (including the final nul \0 character of course).

If it is only temporary, you can either use a local variable where it'll be allocated on the stack or dynamically allocate memory on the heap if it wants to live longer than just the current function. Dynamic allocation is more fiddly but unlike stack allocation you can set array sizes computed at runtime.

Getting your head around C memory allocation, and arrays and pointers, which are deeply entwined in C, is not easy as a concept to begin with, especially if you've come from a Java or C# background where it's largely hidden from you, but it's a bit like learning to ride a bike, once you've figured it out you'll wonder what all the fuss was about. But it might take a little while for it to become second nature.

[katzohki]

OK, so regarding it being as large as the source string I understand that's what should be done. In this case, the tricky bit is that the message stored in EEPROM will be a string of unpredictable length (and reconfigured via serial comms at certain times). If I knew the length ahead of time it would be one thing, but since I don't... well I guess that's the bit I'm having trouble with. Right now I have message defined as a global variable (of size 1 as pointed out). I guess I could check the size of MSG at runtime (already doing this anyway) and set up the message array with that size of "len". Would that be a good way to go? It would also provide the chance to check MSG for max length.

[IanB]

Is it necessary to copy the source string out of EEPROM into RAM before using it? If you only want to display the string and not modify it it should not be necessary to copy it.

[helius]

in C99 you can use variable-length arrays (VLAs). So instead of declaring your char message[] in global scope, you could declare it inside a function.

Code: [Select]

int main() {
    char message[strlen(MSG)+1];
    strcpy(message,MSG);
....
}
This would let you make a buffer of whatever size is needed to hold the data that's currently in EEPROM. You can also use alloca() in compilers that don't support C99:

Code: [Select]

int main() {
    char *message=(char *) alloca(strlen(MSG)+1);
    if (message!=0) strcpy(message,MSG);
.....
}

The way you have written it with memcpy and sizeof, however, doesn't permit the size to change at runtime: that length is fixed by the source code.

[djacobow]

Whether you want to use alloca/VLA or whether you want to used a fixed buffer (that may be too small) depends on what you expect to happen at runtime and how you want to deal with the error conditions.

For example, with the fixed buffer, if the incoming message is too big, you might just copy the portion that fits into your buffer. You don't get the whole message, but you don't get a crash.

With alloca, if the message is very large, alloca itself will fail and you'll get a null pointer to work with. If you don't check the return value and try to use it, you'll have an instant crash (if you're lucky). On the other hand, if you do check the return value, you have to decide what course of action to take. Punt? Notify the user, etc?

Actually, reading the man page for alloca, it's even worse than that. Alloca actually just gives you a pointer to an extended stack frame. If the stack frame cannot be extended to the requested size, you don't even get a null return value to check. You get undefined behavior. That is, it seems you cannot even theoretically write a correct program this way. And, again, in the embedded world where you are often working with threads with microscopic stacks, this is not very theoretical.

Memory management is unpleasant, tedious work. Embedded systems with very limited resources and external inputs that can very in size without restrictions do not make a happy marriage.

in C99 you can use variable-length arrays (VLAs). So instead of declaring your char message[] in global scope, you could declare it inside a function.

Code: [Select]

int main() {
    char message[strlen(MSG)+1];
    strcpy(message,MSG);
....
}
This would let you make a buffer of whatever size is needed to hold the data that's currently in EEPROM. You can also use alloca() in compilers that don't support C99:

Code: [Select]

int main() {
    char *message=(char *) alloca(strlen(MSG)+1);
    if (message!=0) strcpy(message,MSG);
.....
}

The way you have written it with memcpy and sizeof, however, doesn't permit the size to change at runtime: that length is fixed by the source code.

[Howardlong]

in C99 you can use variable-length arrays (VLAs). So instead of declaring your char message[] in global scope, you could declare it inside a function.

Code: [Select]

int main() {
    char message[strlen(MSG)+1];
    strcpy(message,MSG);
....
}


Well there you go, you can teach an old dog new tricks, how widely is this implemented for microcontroller compilers?

Another option is strdup(), which performs the malloc() for you, but doesn't absolve you from the responsibility of freeing it when you've finished.

Depending on the implementation, some microcontrollers sometimes have addressing restrictions on flash and/or eeprom based objects. The OP hasn't mentioned what device this is for.

One point about using dynamic memory on resource restricted microcontrollers is that it's quite common to avoid allocating memory randomly in the middle of your code at all, just allocating what's necessary at reset. Not only does this simplify things, it also means you can save you from memory leaks and indeterministic operation from fragmentation. As a general rule, I avoid it, in practice it's fairly rare in smaller embedded systems that you need to use the heap dynamically.

[obiwanjacobi]

Depending on the implementation, some microcontrollers sometimes have addressing restrictions on flash and/or eeprom based objects. The OP hasn't mentioned what device this is for.

+1
First thing that went through my mind.
What does 'eeprom' do, exactly?

In general you cannot access different memory spaces (RAM/flash/eeprom) without some sort of API support (usually some asm that does the magic).

[katzohki]

It looks like declaring the RAM array with the correct length might be the way to go. I'll try to answer the questions as best I can. I'm using XC8, which I don't know if it supports C99, but I'll look at the compiler manual.

Part: PIC12LF1840T39A     (basically a 12LF1840 for programming purposes)

The EEPROM stores a data message, along with some configuration data. It will be written at programming time, but in the final application will also be able to be rewritten via serial comms (some USB to UART chip probably). This doesn't need to happen very often. It needs to be maintained during power-off and so I thought it best to keep it in EEPROM rather than in program memory.

The only reason I'm trying to copy the data into RAM is because every time I get a new byte it has to read the EEPROM. Due to the extra time to read out of the EEPROM I thought "this is a bad thing," but maybe it's not. I might be able to be convinced I don't need to do this, but at this point I feel I may as well finish up just for my own education.

Application wise, the stored data is sent serially and then sleeps for a while before repeating.

By the way, storing and retrieve variables from the EEPROM was way easier than I expected in XC8! Just declare it as eeprom type, very easy.

[Howardlong]

You're in a very constrained device as you probably know, I wouldn't even try to use dynamic memory (not even sure it's supported on that device, it only has 256 bytes of RAM!).

There's also no proper stack in C terms, just a procedure PC return stack in hardware, although XC8 makes every attempt to make it look like there is one for C portability.

In view of this I'd make every attempt to statically allocate, if indeed you need to: there is quite a lot of jiggery pokery that XC8 does to make EEPROM access transparent, things like pointers and even strcpy works if the eeprom is source (not dest). This is a double edged sword, though: on the one hand it makes things readable, on the other it hides all the nastiness you have to do so it might hide things that you might want to be more obvious to you.

[obiwanjacobi]

You do not ever ever ever want to use dynamic memory allocation in small MCU devices. It introduces way too much overhead and complexity.

If you cannot 'display' (your code said sendMsg) the text directly from eeprom, you can always copy and send it over one character at a time. In a resource-restricted device you sometimes have to make do...

[katzohki]

Alright, yeah I'm leaning more towards not bothering to copy out of eeprom. Thanks guys!