Hey, I am writing a networking application. One very annoying part of it is that I am not able to write a problem-free serial read then return string function.
This is the function I am using and it causes major memory leaks. Since I would like to run my program indefinitely, I would not like ANY memory flaw.
I used to use strings rather than char* but then I read that string is buggy and is known to have memory leaks. However, using this code even with char* does not conserve any memory, even when I free() the returned char* after its use (in another scope, of course).
Can someone please help me out here and see why it errors? If not, can you please paste your own general read-serial-and-not error code? Thanks.
#define MAX_READ_XBEE 512
char* CreadXbee(int timeoutTime){
xbee.listen();
char xbeeOut [MAX_READ_XBEE] = ""; //#defined above
memset(xbeeOut, 0, sizeof(xbeeOut));
float startTime = millis();
while (!xbee.available()) {
if(millis()-startTime > timeoutTime){
return "";
}
}
int howManyElem = 0;
while(xbee.available()){
xbeeOut[howManyElem] = (char) xbee.read();
delay(10);
howManyElem ++;
}
//free(xbeeOut + howManyElem);
xbeeOut[howManyElem] = 0;
// delay(1000); Serial.print(xbeeOut); Serial.print(" Got, which is this big: "); Serial.print(strlen(xbeeOut));Serial.print(" Which should be this big: "); Serial.println( howManyElem );
return xbeeOut;
}
This is wrong in more ways than than. For one thing, you are allocating 512 bytes of precious memory on the stack. Then you return a pointer to it, which has now gone out of scope.
I would like to receive strings small and large with Readxbee function. Thus, I put 512. But this one, for one only uses 25 bytes. However, I would still need the function to at least be able to read 512.
Any help? Thanks
got points to the stack. At this point, it is pointing to garbage.
free(got);
Freeing a pointer to memory you didn't allocate. is a great way to corrupt memory.
I would like to receive strings small and large with Readxbee function.
You need to decide what defines a large string, and pass an array of that size to the function. Have it return a value indicating whether the array contains all the data, or not.
However, I would still need the function to at least be able to read 512.
Local variables in functions are pushed onto the stack. When you return from the function, they are popped off of the stack and the space is immediately available.
While the pointer points to space that may be overwritten at any time, there is nothing that says that this does happen. On the other hand, there is nothing to say it doesn't. What you are doing is relying on it not happening, which is a poor programming practice.
Am I not copying the pointer?
No. You are copying the data from the memory location that the pointer points to. It is that memory that can be overwritten at any time - like the next time a function is called. It's hard to call memcpy() without calling a function.
It will occasionally send large amounts of data in a packet. All this data is stored in a struct which is cast into a char* and then the null characters are replaced with another character so that the char* is valid.
On the decoding side, the reverse is done and the struct is once again created. My code runs correctly once and a half times, allocating that 512b, decoding it, then allocating another 512b , then succumbs to major memory errors before a second decoding.
Pauls so I do need to read everything at once, as processing it individually will ruin the struct. Is there any way like if I get a global variable to always use and always return the pointer to that variable?
davidl_i:
Pauls so I do need to read everything at once, as processing it individually will ruin the struct.
That's simply not true. You can read them seperately, then only process the data when you receive something that indicates there is no more data.
davidl_i:
Is there any way like if I get a global variable to always use and always return the pointer to that variable?
There is no point in returning a global variable, just return something to indicate if there is valid data in the global variable and the code outside of the function can utilize the global variable.
davidl_i:
I would like to receive strings small and large with Readxbee function. Thus, I put 512. But this one, for one only uses 25 bytes. However, I would still need the function to at least be able to read 512.
Any help? Thanks
You may wish to consider a more simplistic implementation; say perhaps the way it is done with GPS data. Here is my implementation of serial GPS read based loosely on Limor Fried (Adafruit) GPS library
char nmea[120];
// ...............................
c = UART_UartGetChar(); // Get received character or null
if (c)
{
if(c == '
) { // $ start of NMEA sentences
for(k=0; k<5; k++) // need 5 characters for sentence type
{
LED_Write( ~LED_Read() ); // flicker LED for activity
do {
c = UART_UartGetChar();
}
while (! (c));
nmea[k] = c; // G + P + R + M + C
// sprintf(buffer + k, "%c", c) ; // only for debug
}
LED_Write( LOW ); // LED off
if (strstr(nmea, "GPRMC"))
{
do {
do {
c = UART_UartGetChar();
LED_Write( ~LED_Read() ); // flicker LED
} while (!(c));
nmea[k] = c;
++k;
} while ( !( c == '*' ) && k < 120) ; // marker or end of input array?
LED_Write( LOW ); // LED off
// Inspiration: Limor Fried's Arduino GPS lib
char *p = nmea;
p = strchr(p, ',') + 1; // position after 1st comma
// float timef = atof(p);
// uint32_t time = timef;
uint32_t time = atoi(p);
hour = time / 10000;
minute = (time % 10000) / 100;
seconds = (time % 100);
// output to GLCD
// ......................................
davidl_i:
Pauls so I do need to read everything at once, as processing it individually will ruin the struct....
Ruin the struct? Like ruining the washing? No.
As I pointed out in another recent thread, whether you believe it or not, you can process a byte at a time using a state machine, and move the information "on the fly" into variables. 512 bytes is simply not necessary.