access violation?

Started by
7 comments, last by Zahlman 15 years, 5 months ago
ive been stuck on this for a while:


char number[15]={'0','1','2','3','4','5','6','7','8','9','A','B','C','D','E'};

char *change(char* string)
{
	char* str = string;		
	int a = 15;
	int x=0;
	while (x!=2)
	{
		
		if (string[x]==number[a])
		{
			a = abs(a-15);
			str[x] = number[a];
			x++;
			a=15;
		}
		a--;
	}
	return str;
	
}
 
Iv'e tried a few different variations, but no matter what i do, i get an excepio:" Unhandled exception at 0x00411ed0 in encrypt.exe: 0xC0000005: Access violation writing location 0x00418910." im passing in a hexadecimal value (currently just trying 'CE'). i have no idea what's wrong.
Advertisement
number[15] is a ZERO indexed array, therefore the legal range of values you can look up with are 0-14. In your sample you set a to 15 and use that to attempt to index into the 16th element of number which is 15 long.
Arrays have 0-based indices, so for an array of size 15 valid indices are in the range [0,14]. You're looking at number[15], which is invalid.
Quote:
Unhandled exception at 0x00411ed0 in encrypt.exe: 0xC0000005: Access violation writing location 0x00418910


In addition to the above notes, be careful with string literals. Writing to them will probably cause your program to segfault. Many compilers will place such literals in read only memory. Use an array initialised with the literal in the calling code:
int main(){    // don't do this:    change("CE");    // use this:    char string[] = "CE";    change(string);}

Either that or make an explicit copy of the argument inside the function, for example using strdup(). This way you also have to manage the returned value, to correctly free() it.

A final solution (and a common C idiom) would be to pass in a "write" array, as well as the read array:
void change(const char *read, char *write){    // ... }


If you are programming in C++, ignore the above and use std::string [smile].
well im trying to kinda mess with my own encryption kinda thing. So i won't be passing in actualy strings, just vars.

also turning it to 14 didn't fix it. i should've mentioned that my error is at

str[x] = number[a];
Using my first suggestion, your original code ran on my machine (though it may access memory out of bounds):
#include <iostream>#include <cmath>char number[15]={'0','1','2','3','4','5','6','7','8','9','A','B','C','D','E'};char *change(char* string){	char* str = string;			int a = 15;	int x=0;	while (x!=2)	{				if (string[x]==number[a])		{			a = abs(a-15);			str[x] = number[a];			x++;			a=15;		}		a--;	}	return str;}int main(){	char copy[] = "CE";	std::cout << change(copy) << '\n';}


Just using std::cout << change("CE") << '\n' caused a similar segfault to occur.

A couple of other questions:

* Are you using C or C++?

* Are you writing this for fun, or are you actually trying to encrypt something?

* Is there any reason you are not including 'F' in your table?
How about this:
char number[15]={'0','1','2','3','4','5','6','7','8','9','A','B','C','D','E'};void change(char *string){  int x = 0;  while ( string[x] )  {    for ( int i = 0; i < 15; ++i )    {        if ( string[x] == number )        {           int rotIndex = abs(i-15);           string[x] = number[rotIndex];        }    }    x++;  }}


your problem could be any number of things. from x >= strlen(string) to a < 0 (the case if string[x] isn't found in number.
Note also that char *str = string; DOES NOT COPY string. it just makes another pointer into string. so your line str[x] = number[a] is the same as string[x] = number[a].
C++, just for fun, and im really not sure why I didn't include it, im just trying to get this running. i really don't care about that for now anyway.

also i was getting this error when x=0; ima check out the examples you guys gave.
Let's pretend for a second that we're using C.

0) As noted, an array declared with [15] has 15 elements, and the valid indices for that array are 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, and 14. Count those: there are 15 possibilities. Note that the last possibility is 14, not 15. That's because we started counting at 0.

1) No copy of the string is actually made at any point, despite what you might think. All that is copied is the pointer to the beginning of the string. 'str' and 'string' always hold the same value: a memory location at which some text is located. Thus, no good comes from making 'str' at all; we eliminate this and change all 'str's to 'string's.

2) Since there is no copy being made, all the changes we make are to the pointed-at text. There is no use in returning anything, because the string doesn't move in memory. The calling function doesn't need to be told where the resulting text is, because it simply replaces the provided text.

3) Consider what happens if a symbol in the input text is not found in 'number'. 'x' can never be incremented, because the 'x++' line occurs inside the if-block, which is only triggered if we find a match, which we just said won't happen. Eventually, 'a' reaches the value of -1, and ceases to be a valid index into the array. We should guard against this.

4) Consider the purpose of the loop: we want to look at each character in succession. That is, we are repeating an operation 'for' each character. Thus, we should logically expect to use a 'for' loop.

5) For each character, we must start the 'a' counter anew. It makes sense to use a separate loop for this. This also agrees with the principle of restricting variable scope: we don't need 'a' outside the outer loop, so we shouldn't have to declare it out there.

6) There's no particular reason to count backwards with 'a'. Counting forwards is more easily understood.

7) If you have an expression like 'abs(x - y)', and you know x <= y always, then you can conclude that (x - y) <= 0 always; therefore abs(x - y) == -(x - y) always; therefore abs(x - y) == (y - x) always.

8) Don't use magic numbers. There are tricks you can use to count the size of an array at compile time.

const char number[] = {'0','1','2','3','4','5','6','7','8','9','A','B','C','D','E'};// The number of elements in an array (keep in mind that you can NOT just pass// an array to a function and make it work, because you don't really receive// the array but just a pointer, unless you are very careful, and in those cases// you know the number of elements anyway) can be found as the size of the// array divided by the size of the element type. But sizeof(char) == 1,// *by definition*.const int number_count = sizeof(number);void change(char* string) {	int x;	for (x = 0; x < 2; ++x) {		int a;		for (a = 0; a < number_count; ++a) {			if (string[x] == number[a]) {				// If a is in (0..number_count), then number_count - a must also be.				string[x] = number[number_count - a];				break; // so that we don't make another change.			}		}		// The break statement above would land *here*.		// If there weren't any matches, we still get to this point, and		// just leave the character unchanged and go to the next one.	}}


Finally, to make it possible to operate on an input string of any length, we simply loop until we find a null terminator. The easiest way to write this loop is to advance a pointer directly (instead of advancing a counter and using it as an offset), thus:

const char number[] = {'0','1','2','3','4','5','6','7','8','9','A','B','C','D','E'};const int number_count = sizeof(number);void change(char* string) {	for (; *string; ++string) {		int a;		for (a = 0; a < number_count; ++a) {			if (*string == number[a]) {				*string = number[number_count - a];				break;			}		}	}}


Notice that we don't need to copy the 'string' pointer. Why? Because it was already copied by virtue of the function being called. The caller still has a pointer to the beginning of the string, and that pointer is not modified by our function.

This topic is closed to new replies.

Advertisement