0

The question itself begs an obvious answer. In any case, here's a snippet of my code...

    switch(cSet)...

    case 8:{ //Special Characters
        finalSet = special;
        char* charSet = new char[special.size() + 1];
        charSet[special.size()] = 0; //Append null terminator
        memcpy(charSet, special.c_str(), special.size());
        break;
    }

    case 9:{ //Alphnumeric and Special character
        finalSet = all;
        char* charSet = new char[all.size() + 1];
        charSet[all.size()] = 0; //Append null terminator
        memcpy(charSet, all.c_str(), all.size());
        break;
    }
    ...

Note that finalSet is of type std::string. I'm needing to save it as a character array. After this statement, I call charSet outside of the switch statement:

    for(int i = 0; charSet; i++)
         printf("%s", charSet[i]);

Now, it is obvious that switch statements are conditional, so a variable may not always be declared. So Visual Studio 2012 is throwing the error "charSet is undefined." The way I have my switch statement structured, though, charSet will always be defined, or the program will exit in the default case.

To remedy this issue, I attempted to declare charSet outside the scope of the switch statement. When I do this, however, for some reason the compiler throws a read access error.

I'm curious as to how I can fix this issue.

Any constructive input is appreciated.

Error code when declaring outside of switch statement:

`Unhandled exception at 0x0F6616B3 (msvcr110d.dll) in cuda_comb.exe: 0xC0000005: Access violation reading location 0x00000061.`
Mlagma
  • 1,240
  • 2
  • 21
  • 49
  • Please show the other error in detail. You should be declaring the charSet outsize the switch statement, as you stated. – OldProgrammer Nov 25 '13 at 20:08
  • Sure, give me a moment... – Mlagma Nov 25 '13 at 20:09
  • You have two different `charSet` variables, declared in different scopes. Any value you assign to `charSet` is lost when execution reaches the `break` statement, and the name `charSet` is not visible outside the enclosing block. And you have a memory leak. – Keith Thompson Nov 25 '13 at 20:11
  • If using C++11, you can just use the `std::string`'s buffer as a modifiable character array to some extent. – chris Nov 25 '13 at 20:11
  • Looks like the condition in your `for` loop is off, you probably want something like `for (char *it = charSet; *it; ++it) printf("%c", *it);` – Andrew Clark Nov 25 '13 at 20:13
  • You are accessing invalid memory. You should be able to set a breakpoint in the debugger and see the offending line. Usually, you have overrun a buffer. – OldProgrammer Nov 25 '13 at 20:16
  • Yes, I figured as much. Like @Kninnug pointed out, it was due to an error in my for loop. – Mlagma Nov 25 '13 at 20:21
  • Yup, `%s` should be `%c` in your `printf` – nonsensickle Nov 25 '13 at 20:22
  • Sorry, it seems you noticed it but I'll say it just in case. You also don't have the correct stop condition in `for(int i = 0; charSet; i++)` try `for(int i = 0; charSet[i]; i++)` – nonsensickle Nov 25 '13 at 20:25
  • How have you declared it? I mean, if you declared it just `char* charSet;` and you're not entering in a case that allocs some memory, you're reading an invalid memory position. Please, check my solution proposal in my answer. – jcm Nov 25 '13 at 20:49

4 Answers4

5
  1. You have to declare charSet outside the switch. When the variable is declared inside the switch/case it will run out of scope when the switch ends, even if the case that declares the variable is executed.
  2. Your for(int i = 0; charSet; i++) loops as long as charSet isn't 0 and increments i every time. Because you don't change charSet inside the for it will always not be 0 so that eventually charSet[i] is out of bounds and gives you the read access error. Change it so that it loops until i is greater than the length of the buffer, or charSet[i] == '0' (the terminating-null). For example:

    for(int i = 0; charSet[i]; i++) 
        printf("%c", charSet[i]);
    
  3. Also change the %s in the printf to %c as you're printing a char, not a char *. Although, as you're printing a null-terminated string anyway, you can avoid the for loop altogether:

    printf("%s", charSet);
    
Kninnug
  • 7,992
  • 1
  • 30
  • 42
2

When you took it out did you try (before the switch):

char* charSet = 0;

and then in all the switch statements remove the char *, so

char* charSet = new char[all.size() + 1];

becomes:

charSet = new char[all.size() + 1];

then your code can even check (after the switch):

if (!charSet)
{
  // handle odd case
}
Glenn Teitelbaum
  • 10,108
  • 3
  • 36
  • 80
2

You're defining charSet on every case block, although you allocated memory on the heap for this array, outside these blocks, charSet reference is undefined as the builder is showing.

Note that, by doing this, if you were not using charSet outside switch block, you will receive no error but you will be leaking memory.

I mean:

switch(cSet)...

case 8:{ //Special Characters
    finalSet = special;
    char* charSet = new char[special.size() + 1]; // <-- charSet definition
    charSet[special.size()] = 0; //Append null terminator
    memcpy(charSet, special.c_str(), special.size());
    break;
} // <-- charSet reference lost, Memory leak

case 9:{ //Alphnumeric and Special character
    finalSet = all;
    char* charSet = new char[all.size() + 1]; // charSet definition
    charSet[all.size()] = 0; //Append null terminator
    memcpy(charSet, all.c_str(), all.size());
    break;
} // <-- charSet reference lost, Memory leak

} // End of switch

    for(int i = 0; charSet; i++)
     printf("%s", charSet[i]); // <-- Error charSet is not defined

Correct solution for this is declaring charSet outside the switch statement so you won't lose the reference:

char* charSet = NULL;
switch(cSet)...

case 8:{ //Special Characters
    finalSet = special;
    charSet = new char[special.size() + 1];
    charSet[special.size()] = 0; //Append null terminator
    memcpy(charSet, special.c_str(), special.size());
    break;
}
...
}

if(charSet)
   printf("%s", charSet);

Note that, as you're printing an array with "%s", you can directly use charSet.

jcm
  • 2,568
  • 14
  • 18
  • Umm, you too copied his error. `printf("%s", charSet[i]);` should be `printf("%c", charSet[i]);` shouldn't it? As well as you don't have a stop condition for the loop... Try `for(int i = 0; charSet[i]; i++)` instead. – nonsensickle Nov 25 '13 at 20:24
  • @nonsensickle Don't worry, it was just a fast copy paste from original source code. After reviewing, I thought that, for code cleanliness, it should be better just to use a single `printf`. – jcm Nov 25 '13 at 20:36
0

From what I understand your code looks like this:

 switch(cSet){    
        case 8:{ //Special Characters           
           char* charSet  = new char[special.size() + 1];
           //code
           break;
          }
         case 9:{ //Special Characters           
           char* charSet = new char[all.size() + 1];
           //code
           break;
           }
        //other cases
     }

   for(int i = 0; charSet; i++)
         printf("%s", charSet[i]);

In this case even though you define charset for each case, you restrict the scope of charset to each case where it's defined - which you already figured out and moved the pointer declaration outside the switch - like:

char* charSet;
switch(cSet){    
       //code
}

The error you are getting basically says you are reading from an invalid memory location. Which can happen if you don't assign the pointer to a dynamically allocated memory location. Even though you seem to allocate memory for charSet for all cases of the switch statement, don't forget you have a default case as well.

Hope it helps.

Pandrei
  • 4,843
  • 3
  • 27
  • 44