Problem using an array inside a switch statement

Question

I am having a problem when using a switch statement to check for something inside an array. I am using an int as my switch, then I compare the character arrays using strstr. So it reads like this:

void function(char *buffer, uint32_t size){
  char *p=(char*)malloc(compareSize); //compareSize is the size of the text to be compared
  #define TXTCOMPARE1 (char*)"Sample Text 1"
  #define TXTCOMPARE2 (char*)"Sample Text 2"
  #define TXTCOMPARE3 (char*)"Sample Text 3"
  switch(intCompare){
    case COMPARE1 :
      p = strstr(buffer,TXTCOMPARE1); break;
    case COMPARE2 :
      p = strstr(buffer,TXTCOMPARE2); break;
    case COMPARE3 :
      p = strstr(buffer,TXTCOMPARE3); break;
  }
  if(p!=NULL) { Serial.println("Success!"); }
  else { Serial.println("No match"); }
    memset(&p,0,sizeof(p);
}

This method does not work and p is always NULL. However, when I write it out without the switch() it compares just fine:

//using the same p declaration as above
p = strstr(buffer,TXTCOMPARE1);
if(p != NULL) { Serial.print("Success:"); }
else { Serial.print("No Match"); }

Why would the switch statement give me different results for what is basically the same function call?

Edit: Actually it seems to break everywhere when I set more than one call to p = strstr(...). When there is only one strstr it works, when two or more are used, it does not...

Have you defined COMPARE1, COMPARE2 and COMPARE3 ? Where is your value assigning statement for intCompare ? — Kugan Kumar
– Kugan Kumar, Commented Sep 10, 2016 at 4:52
Yes, I appologize for leaving that out. But that is almost exactly what I'm doing, except that intCompare is being passed into function(char *buffer, uint32_t size, int intCompare). But when I have multiple cases, p is always NULL — Andrew
– Andrew, Commented Sep 10, 2016 at 6:40
I don't understand three things in this code: 1) Why are you doing a malloc() into p, and then overwriting p before doing a free()? That is not what strstr() needs to work. 2) Why do the TXTCOMPARE strings have (char *) before them? This shouldn't be necessary. 3) Why are you doing a memset(&p,0,sizeof(p));? This is an extraordinarily long-winded way of saying p = NULL; - and then p goes out of scope anyway... — John Burger
– John Burger, Commented Sep 10, 2016 at 8:30
Thanks @JohnBurger. 1) I'm trying to put in an expected size for p from the comparison text I am expecting. 2) The compiler was giving me warnings for converting the strings into char* when defining them, so I cast them as char*. Is there a better way to do this? 3)I thought I was trying to clear the p pointer. — Andrew
– Andrew, Commented Sep 11, 2016 at 2:13

Delta_G · Accepted Answer · 2017-10-07 23:55:15Z

1

One big problem is that you forgot the break statements for those cases. So if it compares in the first one and finds it, you'll still get null because it is going to go look for the third one afterwards.

answered Oct 7, 2017 at 23:55

Delta_G

3,3912 gold badges13 silver badges24 bronze badges

Add a comment |

Andrew · Accepted Answer · 2016-09-11 05:14:38Z

0

I solved this problem, there were a few problems with the code that I found, so thanks for your comments. Basically, it lies in the #define# statements. First, in Arduino it's not such great practice to use #define for strings/chars, only for int. This is because the compiler applies the text every time they appear in the code, so it uses up a lot more memory. The problem here is solved by using static char TXTCOMPARE1 = {"Sample Text 1"};

Another problem was that there was no NULL terminated character at the end of the #define statement, so the program kept looking to the end of the stack when using strstr() and finding no termination, I think I was just getting lucky that it worked occasionally at all.

Furthermore, there is no need to clear p using memset. It needs to be declared as NULL and then reset after the switch by if(p){p = NULL;}.

And now it works!

answered Sep 11, 2016 at 5:14

Andrew

1071 silver badge13 bronze badges

Good that you found it - and thank you for the caution about #define and strings. Note that you should have used static const char TXTCOMPARE1[] = "Sample Text 1";, and also that if (p) { p =NULL; } wastes an if. Just do p = NULL; - it's smaller and faster. Oh, and completely unnecessary. p is about to disappear anyway when it goes out of scope: no need to zero it out.

John Burger
– John Burger

2016-09-11 05:51:16 +00:00
Commented Sep 11, 2016 at 5:51
1

Another problem was that there was no NULL terminated character at the end of the #define statement - that's not true. It's hard to tell from your snippet but a #define just does a textual replacement.

Nick Gammon
– Nick Gammon ♦

2016-10-12 01:01:32 +00:00
Commented Oct 12, 2016 at 1:01
2

There is no need to malloc memory for p, just declare it as a char *p = NULL; strstr then returns a pointer to the existing strings. All the malloc line is doing is leaking memory.

Andrew
– Andrew

2017-01-10 09:32:19 +00:00
Commented Jan 10, 2017 at 9:32
The statement "This is because the compiler applies the text every time they appear in the code, so it uses up a lot more memory" is wrong. No matter what declaration you use (#define, const char), the memory usage is the same. The C compiler take care of reusing the string when it appears for second time. Wrote a little test program and see how much RAM/program memory it use.

user31481
– user31481

2017-08-08 19:37:21 +00:00
Commented Aug 8, 2017 at 19:37

Add a comment |

Stack Exchange Network

Problem using an array inside a switch statement

2 Answers 2

Your Answer

Hot Network Questions

Problem using an array inside a switch statement

2 Answers 2

Your Answer

Sign up or log in

Post as a guest

Related

Hot Network Questions