I did some searching and I think it might be a buffer overflow but I'm not sure. Could someone tell me what's wrong with this?

SafeGuard:: SafeGuard(int agent)
{
for(int index = 0; index < 26; index++)
{
upperCase[index] = 'A' + index;
lowerCase[index] = 'a' + index;
}

for(int alter = 0; alter < 26; alter++)
{
shiftedUpper[alter] = upperCase[(1 + agent) * 9 )% 26];
shiftedLower[alter] = lowerCase[(1 + agent) * 9 )% 26];
}
cout<<"Upper Case: "<<upperCase<<endl;
cout<<"Lower Case: "<<lowerCase<<endl;
cout<<"Shifted upper case: "<<shiftedUpper<<endl;
cout<<"Shifted lower Case: "<<shiftedLower<<endl;
}

======================class header===============

#ifndef CLASS_SafeGuard_
#define CLASS_SafeGuard_
#include <iostream>
#include <string>
using namespace std;

class SafeGuard
{
private:
char upperCase[26];
char lowerCase[26];
int shift;
char shiftedUpper[26];
char shiftedLower[26];
char message[100];
public:
SafeGuard::SafeGuard(int shift);
char *encrptMessage(char message[]);
char *decryptMessage(char message[]);
char *getMessage();
int getShift();
};
#endif

For some reason when I output upperCase it contains the values stored in it as well as the values stored in lowerCase. The same thing for shiftedUpper. What's going on here?

P.S My professor says it has something to do with the memory locations. For some reason in the class putting shift in between two char arrays remedies the problem.


Additional Details
When I added the null character the lowerCase char array won't display anything.

Recommended Answers

All 9 Replies

Member Avatar for r.stiltskin

For some reason when I output upperCase it contains the values stored in it as well as the values stored in lowerCase...

I think you will have to show how you are outputting the arrays for anyone to tell what is going wrong with that.

Also, have another look at this:

for(int alter = 0; alter < 26; alter++)
{
shiftedUpper[alter] = upperCase[(1 + agent) * 9 )% 26];
shiftedLower[alter] = lowerCase[(1 + agent) * 9 )% 26];
}

If, for example, agent=0, shiftedUpper will be an array of 26 A's. Is that really what you want? (Similarly for shiftedLower.)

I think you will have to show how you are outputting the arrays for anyone to tell what is going wrong with that.

Also, have another look at this:

for(int alter = 0; alter < 26; alter++)
{
shiftedUpper[alter] = upperCase[(1 + agent) * 9 )% 26];
shiftedLower[alter] = lowerCase[(1 + agent) * 9 )% 26];
}

If, for example, agent=0, shiftedUpper will be an array of 26 A's. Is that really what you want? (Similarly for shiftedLower.)

Ok I did this before but the way I fixed it was declaring the variables like this
char[]
int
char[]
For some reason that fixed it. As for how I'm outputting it, I'm not quite sure what you by that but, In the default constructor I'm outputting the base addresses of the char arrays to that I can determine if the variables were initialized correctly.

Member Avatar for r.stiltskin

Sorry, I wasn't paying attention to the rest of the constructor. The problem there is that if you give a char array to cout, it will print everything starting at the beginning of the array and continuing until it finds a null char '\0' (or simply a zero byte). So if you want to do

cout << upperCase;

the last byte of upperCase should be a '\0'. Otherwise you have to print it by looping through the array one element at a time.

Your char[], int, char[] solution probably worked because the value of the int happened (luckily) to be 0.

My other comment was about how you calculate the values for shiftedUpper and shiftedLower, not about the output.

Sorry, I wasn't paying attention to the rest of the constructor. The problem there is that if you give a char array to cout, it will print everything starting at the beginning of the array and continuing until it finds a null char '\0' (or simply a zero byte). So if you want to do

cout << upperCase;

the last byte of upperCase should be a '\0'. Otherwise you have to print it by looping through the array one element at a time.

Your char[], int, char[] solution probably worked because the value of the int happened (luckily) to be 0.

My other comment was about how you calculate the values for shiftedUpper and shiftedLower, not about the output.

So there isn't an actual problem? So long as I'm careful this won't effect my use of the char arrays right?

Member Avatar for r.stiltskin

Your printing problem is not because there's anything wrong with the arrays. It's because of the way you are printing.

So I guess that's a "yes" to your last question.

Since you are using upperCase and lowerCase as character arrays rather than strings, you need to output each value individually.

If you want to use them as a string, you need to remember that as string always ends with \0, so they need to 27 characters long.

Ok so me and my professor went into this in a little more detail. From what he saying in each function allocated memory for each identifier is kept in the same space (if that's the correct way to say it.)

ex I have an int, and 2 char[]
int x = 4
char a1[26] = 'A' - 'Z' (assigned using a for loop)
char a2[26] = the shifted characters.
Ex.)
|00|00|00|04|A|B|C|D|E|F|G|H|I|J|K|L|M|N|O|P|Q|R|S|T|U|V|W|X|Y|Z
|D|E|F|.......|D|

The \0 character that supposed to be included is overwritten and the array doesn't know when the end comes. Also when I manually insert a null character I lose the shifted char array. SO with this being said I have some questions.

1.) When memory is allocated for identifiers, do the identifiers declared in the same block share the same space?
2.) How do I get the compiler to allocate identifiers in separate memory spaces?

for(int i = 0; i != 27; i++)
  {
     if(i < 26)
      letters[i] = 'A' + i;
      
     if(i == 26)
      letters[i] = '\0';
  }
  
  for(int s = 0; s < 27; s++)
  {
     if(s < 26)
      shifted[s] = letters[(s + shift) % 26];
      
     if(s == 26)
      shifted[s] = '\0';
  }

Using this method I'm able to stay safely in my arrays. Using the previous method the null character wasn't given a chance to be inserted into the array. I'll post my findings so that anyone else having this problem will know what to do and how to prevent this.

Member Avatar for r.stiltskin

The \0 character that supposed to be included is overwritten and the array doesn't know when the end comes. Also when I manually insert a null character I lose the shifted char array. SO with this being said I have some questions.

You are confused. The null (\0) character doesn't get put there magically by the compiler. If you want to be able to treat that char array as a string, YOU have to allow space for the null character, and YOU have to put it there. If you want to be able to print it just by writing cout << letters << endl; , you should do this:

char letters[27];
int i;
for(i = 0; i < 26; i++) {
    letters[i] = 'A' + i;
}
letters[i] = '\0'; // here, i==26
cout << letters << endl;

1.) When memory is allocated for identifiers, do the identifiers declared in the same block share the same space?
2.) How do I get the compiler to allocate identifiers in separate memory spaces?

You are confused here as well. Your professor probably said something to the effect that the identifiers declared within a particular block of code are only accessible while that block of code is executing. Even your example shows that they don't share the same space -- if they did, the second array would have completely overwritten the first one. Instead, as your example shows, the second array follows immediately after the first one -- as you would logically expect. So you don't have to do anything special -- each identifier gets its own memory space automatically. But you have to be sure that you are allocating ENOUGH space.

As to the "solution" you posted in Post #9, it is incorrect. You declared each of the arrays to be 26 chars, so, for example, the upperCase array runs from upperCase[0]-upperCase[25]. If you write a '\0' to upperCase[26] you are writing in memory that doesn't belong to that array. You are writing it in memory space that belongs to the NEXT variable that you declared. So if the next variable is the lowerCase array, then the space that you are calling upperCase[26] is REALLY lowerCase[0]; when you assign letters to lowerCase, the 'a' will overwrite the '\0' and you have the same problem as before. Or if you switch things around and declare int shift; between the two char arrays, then either the value assigned to shift will overwrite the '\0' or the '\0' will overwrite shift, depending on which you do last. Instead, the solution is simply to allocate 27 bytes to each array instead of 26, using the last byte of each array to hold the '\0', as in my example above.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.