Help with char* queue

Projects Programming
1

So now that my memory leak is fixed I am still having issues with my simple 12x position message queue.

I am just trying to move the pointers around in order to update the list but more than the last pointer is being updated and I cannot see why. Any help would be appreciated.

#define SCREEN_BUFFER_SZ 12
char *tftBuffer[SCREEN_BUFFER_SZ]={0};

initBuffer(tftBuffer);

pushBuffer(tftBuffer,strcpy_P(buffer, PSTR("Boomer online @ 10.0.0.177")));

void initBuffer (char** data) {
	
	for (int i=0; i < SCREEN_BUFFER_SZ; i++) {
		data[i]=" ";
	}
}


void printBuffer (char** printData) {
	
	int x, y;
	
	for (int i; i < SCREEN_BUFFER_SZ; i++) {
		y = 70 + (10*i);
		myGLCD.setFont(SmallFont);
		myGLCD.setBackColor(VGA_BLACK);
		myGLCD.setColor(VGA_WHITE);
		myGLCD.print((char*) printData[i], 20, y);
	}
}

void pushBuffer (char** bufferData, char* str) {
	char* localBuffer;
	char* localBuffer2;
	int x,y;
	
	// clear out previous characters	
	for (int i; i < SCREEN_BUFFER_SZ; i++) {
		y = 70 + (10*i);
		myGLCD.setFont(SmallFont);
		myGLCD.setBackColor(VGA_BLACK);
		myGLCD.setColor(VGA_BLACK);
		myGLCD.print((char*) bufferData[i], 20, y);
	}
	
	for (int i=SCREEN_BUFFER_SZ-1; i>=0; i--) {			
		if (i==11) {	
			// insert string @ n=11
			localBuffer2=str;	
	    } 
		// save existing data @ n
		localBuffer=bufferData[i];
		// insert n-1 data @ n
		bufferData[i]=localBuffer2;
		// update n-1 data
		localBuffer2=localBuffer;		
	}
	printBuffer(bufferData);
}
2

Can you post a complete (minimal) sketch so we can try to reproduce the bug?

3

Here is the complete code:

4

I realize that a few of the for loops do not set i to an initial value, I fixed that in my code and it had no impact. I think they are initialized to 0 it seems by default.

5

That is a quite a lot of code. What does it say your RAM usage is? Also what board are you using?

6

mkulikow:
I realize that a few of the for loops do not set i to an initial value, I fixed that in my code and it had no impact. I think they are initialized to 0 it seems by default.

I wouldn't rely on that.

7

Board is a Mega with Seeed TFT running UTFT and an Ethernet shield. Using ~ 2M of SRAM.

I was just trying to write a 12x message queue that would display the 12x strings as a list. I would then push a new string into position 11 and shift all the other pointers up one. Clearing out the old strings first in black, then the new strings in white.

It seemed like it should of been simple.

8

I was just trying to write a 12x message queue that would display the 12x strings as a list.

Is there a limit to the number of characters in a string? If so, a static 2D array would be far simpler to deal with.

9

I was also toying with padded arrays, but I figured that the pointer shuffling would of taken care of all of that.

I can try to write the code using tftBuffer[12][36] and clear out both dimensions manually.

10

Although the 2D array approach would not help the fact that the push function is corrupting the current pointer array. The same logic would be needed to shift the strings up in the list.

11

Post your code as an attachment to the forum, not in the rubbish bin.

12

Here is the sketch...

webserver_boomer.ino (20.9 KB)

13

mkulikow:
Any help would be appreciated.

I suspect you need add a -1 in your for conditions but you are trying to do way too much in one function.

Have a look at the example below.
Note:

  1. The listPushUp function does what it says and nothing more.
  2. Everything the function needs to perform the 'pushUp' operation, is provided by the function parameters
  3. The operation performed on the list, is separated from the action 'print,' which is performed with the list.
/* list Iterator example */

//create a list of strings
char string1[] = "string1";
char string2[] = "string2";
char string3[] = "string3";
char string4[] = "string4";
char string5[] = "string5";

//and a container to hold them
uint8_t NUM_STRINGS = 5;
char* strings[] = {string1, string2, string3, string4, string5};

void setup(void) {
  Serial.begin(9600);
}

void loop(void) {
  //iterate the list
  listPrint(strings, NUM_STRINGS);  

  //push the top item onto the bottom of the list
  char* nextString = strings[0];
  listPushUp(strings, nextString, NUM_STRINGS);

  waitInput();
}

//push a newItem onto the bottom of the list
// shift the existing items up
// return the oldItem removed from the top
char* listPushUp(char** list, char* newItem, uint8_t items) { 
  char* oldItem = list[0];  
  
  uint8_t i = 0;  
  do{
    list[i++] = list[i +1];
  } while (i < items -1);

  list[i] = newItem;
  return oldItem;
}

//iterate the list
void listPrint(char** list, uint8_t items) {
  Serial.print("---list---\r\n");
  for(uint8_t i =0; i < items; i++) {
    Serial.print(i);
    Serial.print("\t");
    Serial.println(list[i]);
  }
  Serial.println("--end list--");
} 

//pause for serial and dump the input
void waitInput(void) {
  char ch;
  while(! Serial.available());  
  while(Serial.available())  ch = Serial.read();  
}

You could use a for...next loop in the pushUp but the index value 'i' would need to be declared outside of the construct, so it remains in scope for the final assignment. Hence, I find the do...while construct more intuitive in this case.

  uint8_t i = 0;  
  for (; i < items -1; i++) {
    list[i] = list[i +1];
  }
  list[i] = newItem;
14

Using your methodology the same behavior exists, the new string being pushed onto the stack is somehow propogating into other items on the list.

15

Also, both of your functions take in a char** so whether the operation is "on" or "with: the list, the same item is passed in. I was attempting other things like taking in the pointer to one function (char*) and the list to the other (char**) but the same behavior exists, the list gets messed up after the 2nd or 3rd push.

16

mkulikow:
Using your methodology the same behavior exists, the new string being pushed onto the stack is somehow propogating into other items on the list.

My apologies. I made a typo in the pushUp function,

char* listPushUp(char** list, char* newItem, uint8_t items) {
//..
  strings[i] = newItem; // <-- was referring to a global, doh!

  //should have been
  list[i] = newItem;
//..
}

I have edited my previous post and corrected the faux pas.

Start a new sketch, copy and paste the example from my previous post, compile, upload and open the serial monitor, set the line ending to Newline. Each time you see "--end list--" in the serial monitor, click the send button. The output should look like this

---list---
0	string1
1	string2
2	string3
3	string4
4	string5
--end list--
---list---
0	string2
1	string3
2	string4
3	string5
4	string1
--end list--
---list---
0	string3
1	string4
2	string5
3	string1
4	string2
--end list--
---list---
0	string4
1	string5
2	string1
3	string2
4	string3
--end list--
---list---
0	string5
1	string1
2	string2
3	string3
4	string4
--end list--
---list---
0	string1
1	string2
2	string3
3	string4
4	string5
--end list--

Keep clicking send and the list will cycle again. Can you get that far?

Also, both of your functions take in a char** so whether the operation is "on" or "with: the list, the same item is passed in.

Yes. Conceptually speaking, there is the container (the list of strings) and there are the items in the container (the individual strings).

Physically speaking. 'string1' is the address of an array of chars. 'strings' is the address of an array of pointers, to char. Any function which iterates through 'strings' will need to take a char** and know how many pointers are in the 'strings' array.

So in this case, the type of the list is always char** and the type of items in the list is always char*.

A list of pointers to a function, would be a different type of list and a different set of functions to iterate through it.

17

Even having fixed that error on my end, the array is still getting corrupted on my end.

I will try again and see why the push function is corrupting pointers. I have 6M of RAM left on a Mega so I don't think it's that.

18

It would be better if you used memmove or memcpy instead.

19

mkulikow:
Even having fixed that error on my end, the array is still getting corrupted on my end.

I will try again and see why the push function is corrupting pointers. I have 6M of RAM left on a Mega so I don't think it's that.

I compiled and ran the example on a bare Mega. I understand it might not be too convenient but if at all possible, try removing the RAM expansion. You should be able to get the same result as I did. If that is the case, it would point to the RAM expansion introducing the corruption. I haven't used a RAM expansion on an Arduino but in other scenarios, I have had to select an active page of RAM, to work with.

20

Make a "blank" sketch and try this.

void setup()
{
  myGLCD.InitLCD(LANDSCAPE);
  myGLCD.clrScr();
  myGLCD.setFont(SmallFont);
  myTouch.InitTouch(LANDSCAPE);
  myTouch.setPrecision(PREC_LOW);
  myGLCD.fillScr(0,0,0);
}

/* list Iterator example */

//create a list of strings
char string1[] = "string1";
char string2[] = "string2";
char string3[] = "string3";
char string4[] = "string4";
char string5[] = "string5";

//and a container to hold them
uint8_t NUM_STRINGS = 5;
char* strings[] = {string1, string2, string3, string4, string5};

void loop(void) {
  //iterate the list
  listPrint(strings, NUM_STRINGS);  

  //push the top item onto the bottom of the list
  char* nextString = strings[0];
  listPushUp(strings, nextString, NUM_STRINGS);
  delay(1000);
  
  //waitInput(); // Not Needed for this test
}

//push a newItem onto the bottom of the list
// shift the existing items up
// return the oldItem removed from the top
char* listPushUp(char** list, char* newItem, uint8_t items) { 
  char* oldItem = list[0];  
  
  uint8_t i = 0;  
  do{
    list[i] = list[i+1]; // These were most likely the issue the OP was seeing
    i++;                 // -------------------------------------------
  } while (i < items - 1);

  list[i] = newItem;
  return oldItem;
}

//iterate the list
void listPrint(char** list, uint8_t items) {
  myGLCD.print("---list---", 0,0);
  for(uint8_t i =0; i < items; i++) {
    myGLCD.printNumI(i,0,10 + i*20);
    myGLCD.print(list[i],20,10 + i*20);
  }
  myGLCD.print("--end list--",0,200);
} 

//pause for serial and dump the input
void waitInput(void) {
  char ch;
  while(! Serial.available());  
  while(Serial.available())  ch = Serial.read();  
}