Please help on making a better function

Guys, Here is the function I am using:

void memsetBuff(void){
  memset(debugCommand, 0, sizeof(debugCommand));   // Clear contents of Buffer
  memset(debugData, 0, sizeof(debugData));   // Clear contents of Buffer
}

I call it many times when required in a single sketch for the ease and reduction of code [hence the function usage] for repeated bunch of commands. Whenever I need to do a memset to debugCommand and debugData char arrays, I call this function.

Now, over the period, code grown and also another set of char array came, bmsCommand and bmsData. This also requires several callings the same way, means another function, memsetBuff2().

At this stage, I am planning to reduce the code and to make a universal function, memsetBuff(debug) or memsetBuff(bms) and then it should do accordingly. Hope i made myself clear, what I wanted to mean. This way I intend to call a single function, passing a parameter on which one I wish. I may send 2 parameters also. Ideas??

I'm not always that good in function parameters... (I know i can make an integer passing and can make a if then else or switch case for debug=1, bms=2 etc... but trying to avoid that to make code more readable too)

Often, a "debugCommand [0] = '\0';" is all that's needed.

Of course we can't see your code, so I could be wrong.

i often use sprintf() to format output strings printed using Serial.println() and have a global variable "char s [80]"

since "s[]" holds a string, it doesn't need to be cleared before or after each time it is used.

to AWOL's point, just null terminating the cString at the first char (if the buffer is always maintained as a cString) should be enough and you can just pass your buffer name to a function to clear that up if you really need a function.

char b1[] = "Hello";
char b2[] = "World";

inline void emptycStringBuffer(char * const buffer) {
  *buffer = '\0';
}

void setup() {
  Serial.begin(115200);
  Serial.print(F("buffer 1 = \"")); Serial.print(b1); Serial.println(F("\""));
  Serial.print(F("buffer 2 = \"")); Serial.print(b2); Serial.println(F("\""));
  emptycStringBuffer(b1);
  Serial.print(F("after emptying, buffer 1 = \"")); Serial.print(b1); Serial.println(F("\""));
  emptycStringBuffer(b2);
  Serial.print(F("after emptying, buffer 2 = \"")); Serial.print(b2); Serial.println(F("\""));
}

void loop() {}

untested (typed here) but Serial Monitor (@ 115200 bauds) should show

[color=purple]
buffer 1 = "Hello"
buffer 2 = "World"
after emptying, buffer 1 = ""
after emptying, buffer 2 = ""
[/color]

/* possible obsolete function
void memsetBuff(void){
memset(debugCommand, 0, sizeof(debugCommand)); // Clear contents of Buffer
memset(debugData, 0, sizeof(debugData)); // Clear contents of Buffer
}
*/

void memsetBuff(char * const buff1, char * const buff2){
if (*buff2 == NULL) {
memset(buff1, 0, sizeof(buff1)); // Clear contents of Buffer
return;
}
else {
memset(buff1, 0, sizeof(buff1)); // Clear contents of Buffer
memset(buff2, 0, sizeof(buff2)); // Clear contents of Buffer
return;
}
}

void flashConfModeBuff(void){
configTimerElapsed = 0;
memsetBuff(debugCommand, debugData);
}

Done... was a good task from my end and thanks to all of you for helping me out...

Realization: The more you dig, the more you learn and the more your skill will develop.

It may not matter, but be aware that sizeof(buff1) is not the size of your buffer. It's the size of a pointer, likely two.

wildbill:
It may not matter, but be aware that sizeof(buff1) is not the size of your buffer. It's the size of a pointer, likely two.

I beg you sir... please help me to correct it... plz plz plz

:slight_smile: Serial.print() was showing it the opposite somehow... a variable strvar[10] had "30" and after doing this, it was nothing... serial came out blank

And yep... tested it and yes, "2"... so how can we fix it???

You may not need to fix it. As noted earlier in the thread, it depends on how you're populating the buffer. If you're convinced that it's necessary to clear the entire thing with zeros, then you will need to pass the size to the function as well as the pointer.

wildbill:
You may not need to fix it. As noted earlier in the thread, it depends on how you're populating the buffer. If you're convinced that it's necessary to clear the entire thing with zeros, then you will need to pass the size to the function as well as the pointer.

Indeed, that's what i figured out lately... But actually (I know it's no use, but still) I was planning to fill the entire buff. and then I also realized the best way is to pass the length to the func. it should be a int right?? something like:

void memsetBuff(char * const buff1, int buff1size, char * const buff2, int buff2size)

and calling will be like:

memsetBuff(strBuff1, sizeof(strBuff1), NULL, 0) for single buff process and else as usual, strBuff2 to be added.

aq_mishu:
Indeed, that's what i figured out lately... But actually (I know it's no use, but still) I was planning to fill the entire buff. and then I also realized the best way is to pass the length to the func. it should be a int right??

It should really be a "size_t", because that's what sizeof returns.

TheMemberFormerlyKnownAsAWOL:
It should really be a "size_t", because that's what sizeof returns.

A little explanation please...

BTW, doing memsetBuff(strBuff1, sizeof(strBuff1), NULL, NULL) for single string worked the way i was expecting.

sizeof returns a size_t, not an int.

I don't know how to explain it any further.

We seem to be a long way into this topic without seeing any code.

Ho-hum

// The necessary variables for RS232 Communication
char debugStrRS232_rx[rxBufSize];
char debugStrRS232_tx[txBufSize];
char bmsStrRS232_rx[rxBufSize];
//char bmsStrRS232_tx[txBufSize];
char hmiStrRS232_rx[rxBufSize];
//char hmiStrRS232_tx[txBufSize];
char debugCommand[20];    // Arbitrary Value for command size
char bmsCommand[15];    // Arbitrary Value for command size
char hmiCommand[15];    // Arbitrary Value for command size
char debugData[10];       // ditto for data size
char bmsData[10];       // ditto for data size
char hmiData[10];       // ditto for data size
char bmsPayload[60]; //Payload for pong
char hmiPayload[60]; //Payload for pong
//bms commands
  if (strcmp(bmsCommand, "bmsinit") == 0){
    memsetBuff(bmsCommand, sizeof(bmsCommand), bmsData, sizeof(bmsData));
    bmsInit();
  }
//BMS Serial Parser
void bmsSerialParser(void) {
  bmsByteCount = -1;
  bmsByteCount =  bms.readBytesUntil('\n',bmsStrRS232_rx,rxBufSize);  
  delay(5);
  if (bmsByteCount > 0) {
      strcpy(bmsCommand,strtok(bmsStrRS232_rx,"="));
                 
      strcpy(bmsData,strtok(NULL,","));             
  }
  memsetBuff(bmsStrRS232_rx, sizeof(bmsStrRS232_rx), NULL, NULL); //memsetBuff function takes 2 parameters and we have only one. hence second parameter is NULL
  bms.flush();
   
}
void memsetBuff(char * const buff1, int buff1Size, char * const buff2, int buff2Size){
  /* Based on 
   * memset(charString, 0, sizeof(charString));  
   * To Clear contents of Buffer. 
   * Here we are using pointers to get the value of the char array, and getting the size of the actual array using an int parameter.
   * If we use sizeof(buff1) here, then we will ONLY get 2, not the actual size of the char array we are dealing with.
   */
   
  if (*buff2 == NULL) {
    memset(buff1, 0, buff1Size);   // Clear contents of Buffer
    return;
  }
  else {
    memset(buff1, 0, buff1Size);   // Clear contents of Buffer
    memset(buff2, 0, buff2Size);   // Clear contents of Buffer
    return;
  }
}

See reply #1.

Reply #7 tends to reinforce my belief.

TheMemberFormerlyKnownAsAWOL:
See reply #1.

Reply #7 tends to reinforce my belief.

I understand... that small command is enough... just as there is the memset and just as was thinking of filling the array with NULLs, that's why was thinking this weird way...

Now, passed the int value as given code, and found serial.print() gave exact sizes when called from the function (passed value). Whatever, first of all, it's working (aka serving my purpose) so far. Now just looking for detailed explanation for learning and better understanding. The function part... passing values by ref and passing array size by value. Isn't it???

I'm sorry, I don't understand the question.

TheMemberFormerlyKnownAsAWOL:
I'm sorry, I don't understand the question.

I was telling that sezeof(chararray) was sent to the function as a value, i used INT and it gave me proper output (so far I have seen) and array was sent by a reference right??

You are passing all parameters by value. The name of the array is a pointer to the first memory address of the array. A pointer occupies (in your setup) 2 bytes. The size of the array as written is an int (should be size_t) and is a value. Passing by reference requires a special way to write the parameter in the function signature (use of &).

When you call the sizeof operator it returns the size of what the compiler knows at that point un time about its parameter. In the main code it knows it’s an array so you get the number of bytes, but in the function it’s only a pointer and the size of a pointer is 2.

Your function has a bug, you should not test the first byte of the second buffer but just test the pointer itself. And what if the first pointer is null?

void memsetBuff(char * const buff1, size_t buff1Size, char * const buff2, size_t buff2Size){  
  if (buff1 != NULL) memset(buff1, 0, buff1Size);
  if (buff2 != NULL) memset(buff2, 0, buff2Size);
}

As discussed, if you use the buffer as a cString all the time it’s wasting CPU cycles to empty the whole array. As posted before just putting a null char in the first memory location terminates the cString right there so makes it empty and there is no need for passing the size.

void memsetBuff(char * const buff1, char * const buff2){  
  if (buff1 != NULL) *buff1 = ’\0’;
  if (buff2 != NULL) *buff2 = ’\0’;
}

What if you wanted to be able to empty 5 arrays? Or 10? Read about Variadic functions.

J-M-L:
Passing by reference requires a special way to write the parameter in the function signature (use of &).

That's what is even better right?

J-M-L:
Your function has a bug, you should not test the first byte of the second buffer but just test the pointer itself. And what if the first pointer is null?

Good point... but basically I'm considering that when single array, then first one will be the one and hence there is a "no issue" for "me". But your way is even faar better. Liked it...

J-M-L:
As discussed, if you use the buffer as a cString all the time it's wasting CPU cycles to empty the whole array. As posted before just putting a null char in the first memory location terminates the cString right there so makes it empty and there is no need for passing the size.

Now I got the point. Liked that approach too.

So we can just get rid of memset I understand... (I was kind of fallen in love of that function... lol) But no worries. CPU for those tiny mcu is important...

Variadic functions Well, a life saver for sure...