Issue with memmove, memcopy

Created Wed, 27 Jul 2011 23:11:49 +0000 by whoover

whoover

Wed, 27 Jul 2011 23:11:49 +0000

I'm trying to port over some code to an uno32, but it doesn't seem to be working (worked fine on an uno board).

int no_cmd = -1;
volatile int cmdBuffer[] = {no_cmd,no_cmd,no_cmd};

// Removes/Returns the first element in the buffer and presses the specified element to the end of the buffer 
int pressBuffer(int buffer[], int* press) {
  int pop = *(buffer + 0);
  Serial.println("POP:");
  Serial.println(pop);
  int sz = (sizeof(int)*(sizeof(buffer)));
  Serial.println("SIZE:");
  Serial.println(sz);
  memmove(buffer, buffer + 1, sz);
  memcpy(buffer + (sz - 2), press, sizeof(int));
  return pop;
}

void loop() {
  int cmd;
  int sz = (sizeof(int)*(sizeof(cmdBuffer)));
  for (short i=0; i<sz; i++) {
    cmd = pressBuffer((int*) &cmdBuffer, (int*) &no_cmd);
    if (cmd != no_cmd) {
      Serial.print("EXECUTING COMMAND: ");
      Serial.println(cmd);
    }
  }
}

OUTPUT:

POP: -1 SIZE: 16 POP: -1 SIZE: 16 POP: -1 SIZE: 16 POP: 0 SIZE: 16 EXECUTING COMMAND: 0 POP: 0 SIZE: 16 EXECUTING COMMAND: 0 POP: 0 SIZE: 16 EXECUTING COMMAND: 0 ...

sizeof(buffer) is coming back as 4??? Any clues???

whoover

Thu, 28 Jul 2011 00:34:33 +0000

I got it working with the following:

int no_cmd = -1;
volatile int cmdBuffer[] = {no_cmd,no_cmd,no_cmd};

void loop() {
  int cmd;
  int sz = sizeof(cmdBuffer) - 1;
  for (short i=0; i<sz; i++) {
    cmd = pressBuffer((int*) &cmdBuffer, (int*) &no_cmd);
    if (cmd != no_cmd) {
      Serial.print("EXECUTING COMMAND: ");
      Serial.println(cmd);
    }
  }
  int currKey1 = 37;
  pressBuffer((int*) &cmdBuffer, &currKey1);
  int currKey2 = 73;
  pressBuffer((int*) &cmdBuffer, &currKey2);
}
// Removes/Returns the first element in the buffer and presses the specified element to the end of the buffer 
int pressBuffer(int buffer[], int* press) {
  int pop = *(buffer + 0);
  int sz = (sizeof(int)*(sizeof(buffer) - 1));
  memmove(buffer, buffer + 1, sz);
  memcpy(buffer + (sizeof(buffer) - 2), press, sz);
  return pop;
}

rtestardi

Thu, 28 Jul 2011 13:07:18 +0000

I think this might be working by luck...

When you pass an array to a function, I'm pretty sure the size of the array is lost -- the function treats this:

int pressBuffer(int buffer[], int* press) {

Exactly the same as:

int pressBuffer(int *buffer, int* press) {

(At least it does in C -- I assume that's the same with C++.)

So sizeof(buffer) is just the size of a pointer (4 bytes), and has nothing to do with the original declaration of:

volatile int cmdBuffer[] = {no_cmd,no_cmd,no_cmd};

Taking the address of an array here, is also unnecessary (though unharmful), I believe:

cmd = pressBuffer((int*) &cmdBuffer, (int*) &no_cmd);

And in this line, you are doing pointer arithmetic, so your addition is multiplied by the size of the pointee (i.e., what the pointer points to, also 4 bytes), but you're adding a size in bytes from sizeof:

memcpy(buffer + (sizeof(buffer) - 2), press, sz);

I think you'll need to pass in the length of that array explicitly to your function, and then use the length properly, with something like:

#define LENGTHOF(a)  (sizeof(a)/sizeof(a[0]))  // returns number of elements in an array
...
cmd = pressBuffer((int*) cmdBuffer, LENGTHOF(cmdBuffer), &no_cmd);
...
int pressBuffer(int *buffer, int length, int* press) {
  int pop = *(buffer);
  memmove(buffer, buffer + 1, (length-1)*sizeof(*buffer));
  memcpy(buffer + (length-1), press, sizeof(*buffer));
  return pop;
}

whoover

Thu, 28 Jul 2011 22:25:59 +0000

Thanks for the info rtestardi! I hastily threw the code together w/o thinking it through thoroughly.

The following is necessary because cmdBuffer is volatile and the compiler will complain, right?

cmd = pressBuffer((int*) &cmdBuffer, (int*) &no_cmd);

rtestardi

Fri, 29 Jul 2011 02:13:50 +0000

The following is necessary because cmdBuffer is volatile and the compiler will complain, right?

It looked to me like you had a volatile cmdBuffer you were passing to pressBuffer() which was declarted to accept a non-volatile buffer pointer... So I believe the cast will be needed as you had it, and as I also had it. Or maybe I misunderstand the question?

GeneApperson

Fri, 29 Jul 2011 20:26:08 +0000

In ANSI C and C++, the keyword volatile tells the compiler that the value of the variable can be changed by things external to the code sequence in the program where the variable is referenced. This can occur, for example, where the memory address of the variable is actually a peripheral i/o register. It can also occur when a variable is modified by an interrupt service routine.

This is a clue to the optimizer in the compiler not to cache the variable in a register and to reload the variable from memory each time if is referenced.

I may have misunderstood your code example, but I don't see how the array is volatile.

Gene Apperson Digilent

whoover

Sat, 30 Jul 2011 18:36:48 +0000

Thanks for the info GeneApperson and rtestardi. I have a fairly large project that I'm porting from a uno to a uno32. So, I left out part of the code where I'm updating the command buffer inside an external interrupt- thus the volatile declaration. Thanks again for the help!