problem in loop

[pouncer]

char *challengeConcat = "whatever.....";

char hash1[88] = "edp{}e|wxrdse}}u666666666666666666666666666666666666666666666666";

		char incoming[32]; 
		
		int x, y = 0;

		for (x = 0; x < 32; x++, y++) 
		{

			if (0x5C == challengeConcat[y]) 
			{
				++y;

				if (bValidEscape[challengeConcat[y]]) incoming[x] = esc2ascii[challengeConcat[y]];
				else x--;
				
			}

			else incoming[ x] = challengeConcat[y];
		}

and then what i do is
memcpy(&hash1[64], incoming, 32);

but i tried a shortcut to directly put the chars straight onto hash1, but it doesnt work properly, it just overwirtes part of the hash1 with some random garbage chars

any ideas guys?

char *challengeConcat = "whatever.....";
		char hash1[88] = "edp{}e|wxrdse}}u666666666666666666666666666666666666666666666666";
		
		int x, y = 0;

		for (x = 0; x < 32; x++, y++) 
		{

			if (0x5C == challengeConcat[y]) 
			{
				++y;

				if (bValidEscape[challengeConcat[y]]) hash1[strlen(hash1) - x] = esc2ascii[challengeConcat[y]];
				else x--;
				
			}

			else hash1[strlen(hash1) - x] = challengeConcat[y];
		}

[*io*]

First off strlen requires a null terminated string

when you call

hash1[strlen(hash1) - x] = challengeConcat[y];

and x = 0 the null termination will be over written. From then on i presume the behaviour of strlen will be incorrect...

[Paul McKenzie]

char hash1[88];
//...
memcpy(&hash1[64], incoming, 32);

Undefined behavior. You are overwriting memory and this is an error.

You are attempting to stuff 32 characters starting at hash1[64]. The hash array can only hold 88 characters, hash1[0] to hash1[87]. There is no hash1[88], hash1[89], hash1[90], etc. These are out of bounds, and your program will exhibit undefined behavior and possibly crash.

I thought I mentioned that in the other thread?

Regards,

Paul McKenzie

[pouncer]

hmm Paul the first method works perfect for me though

First off strlen requires a null terminated string

when you call

hash1[strlen(hash1) - x] = challengeConcat[y];

and x = 0 the null termination will be over written. From then on i presume the behaviour of strlen will be incorrect...

hey io, how can i fix this problem then?

strlen(hash1) = {0}; ?

[Paul McKenzie]

hmm Paul the first method works perfect for me though

I'll say it again --- Undefined behavior. It may "work", it may crash, it may work for a while then crash, it may work 1000 times straight and crash on the 1001st time, it may work on 10,000 computers and fail on the 10,0001 computer, etc.

In other words, DON'T DO IT.

You cannot overwrite the boundaries of an array as you're doing now with the memcpy -- it's that simple. Whether it "works" or not means nothing. The hash1 has 88 available positions. Overwriting this is an error.

You need to understand that in C and C++, these languages do not give you any automatic error checking to see if you are overwriting the boundaries of an array. This is not like Basic or Java, where the program halts automatically if you make this error. The only indication in C or C++ that you are overwriting memory is when your program acts erratically, or when a customer gives you a call saying your program crashes on his/her computer.

Regards,

Paul McKenzie

[pouncer]

ok ive now changed 32 to 23, because infact the incoming string generated is never more than 22/23

that should fix the outbound problem *if* it ever happens, sorry paul i didnt realise this.

ive actually just decided to do:
memcpy(&hash1[64], incoming, strlen(incoming));

can you help me on the loop problem now mate?

[Paul McKenzie]

ok ive now changed 32 to 23, because infact the incoming string generated is never more than 22/23

that should fix the outbound problem *if* it ever happens,

There is no "if". It *was* happening.

ive actually just decided to do:
memcpy(&hash1[64], incoming, strlen(incoming));

And what guarantee do you have that strlen(incoming) won't overflow the buffer?

can you help me on the loop problem now mate?

It is very difficult to figure out exactly what you're trying to accomplish. Please describe, on a high-level, what this code is supposed to do.

Regards,

Paul McKenzie

[pouncer]

And what guarantee do you have that strlen(incoming) won't overflow the buffer?

ive changed char incoming[32] to incoming[23]

64+23 = 87 < 88 so i dont think it will overflow it. as for the loop problem:

instead of doing:
memcpy(&hash1[64], incoming, strlen(incoming));

i want to directly append the incoming chars to the end of hash1, in my first post the loop works (i generate the string and store it in 'incoming' variable, then use memcpy to append it onto hash1) but i want to take a short cut and add to hash1 throughout the loop

char *challengeConcat = "whatever.....";
		char hash1[88] = "edp{}e|wxrdse}}u666666666666666666666666666666666666666666666666";
		
		int x, y = 0;

		for (x = 0; x < 23; x++, y++) 
		{

			if (0x5C == challengeConcat[y]) 
			{
				++y;

				if (bValidEscape[challengeConcat[y]]) hash1[strlen(hash1) - x] = esc2ascii[challengeConcat[y]];
				else x--;
				
			}

			else hash1[strlen(hash1) - x] = challengeConcat[y];
		}

[Paul McKenzie]

And what guarantee do you have that strlen(incoming) won't overflow the buffer?

ive changed char incoming[32] to incoming[23]

Are you aware how strlen() works? It does not care how big your array is. The strlen() looks for the first NULL termination character and will copy until it encounters it. That means that if the NULL occurs 10,000 characters down the line, regardless of how big your array is, it will copy 10,000 characters. So strlen(incoming) does not guarantee that you will copy only 23 characters.

The function that you're looking for that will copy only a certain number of characters is strncpy().

But overall, this is the danger of using these low-level 'C' functions. That's exactly the reason why std::string was developed.

instead of doing:
memcpy(&hash1[64], incoming, strlen(incoming));

i want to directly append the incoming chars to the end of hash1

What do you mean by "the end of hash1"? The hash1 is an array of 88 chars, so the "end" is hash1[87] and you can't append to it.

Do you mean you want to *replace* hash1[64], hash1[65], ... hash[87] with a certain set of characters?

Regards,

Paul McKenzie

[pouncer]

ahh, i see thanks ill use strncpy now

Do you mean you want to *replace* hash1[64], hash1[65], ... hash[87] with a certain set of characters?

yep!!

[Paul McKenzie]

ahh, i see thanks ill use strncpy now

Do you mean you want to *replace* hash1[64], hash1[65], ... hash[87] with a certain set of characters?

yep!!

int start = 64;
//....
for (x = 0; x < 32 && start < 88; x++, y++) 
{
    if (0x5C == challengeConcat[y]) 
    {
        ++y;
        if (bValidEscape[challengeConcat[y]]) 
        {
              hash1[start] = esc2ascii[challengeConcat[y]];
              ++start;
        }
	else 
            x--;
    }
    else 
    {
        hash1[start] = challengeConcat[y];
        ++start;
    }  
}

Regards,

Paul McKenzie

[pouncer]

thanks paul for the code im just gona test it now, btw:

for (x = 0; x < 32 && start < 88; x++, y++)

is the x++ meant to be there seeing as uve put a new start variable in mate

[Paul McKenzie]

The "start" is just the index into the hash1 array. The loop has to terminate if start >= 88, since you would be overwriting memory if start is >= 88.

I don't know why you associated "x" with "start". All the middle expression does in a for() loop is to tell what must be true to allow the loop to continue.

Regards,

Paul McKenzie

[pouncer]

int start = 64;
//....
for (x = 0; x < 32 && start < 88; x++, y++) 
{
    if (0x5C == challengeConcat[y]) 
    {
        ++y;
        if (bValidEscape[challengeConcat[y]]) 
        {
              hash1[start] = esc2ascii[challengeConcat[y]];
              ++start;
        }
	else 
            x--;
    }
    else 
    {
        hash1[start] = challengeConcat[y];
        ++start;
    }  
}

Regards,

Paul McKenzie

thanks paul, genius, that worked perfectly for me,

could i possible put start++ in the for loop instead:

for (x = 0; x < 32 && start < 88; x++, y++, start++)

[VladimirF]

could i possible put start++ in the for loop instead:

for (x = 0; x < 32 && start < 88; x++, y++, start++)

You can do better than that - I noticed that your "start" gets incremented together with "x", and its range is shorter, and "x" is not used in a loop, so you can remove it and have "start" completely control this loop.

for (int start = 64; start < 88; start++, y++) 
{
    if (0x5C == challengeConcat[y]) 
    {
        ++y;
        if (bValidEscape[challengeConcat[y]]) 
        {
              hash1[start] = esc2ascii[challengeConcat[y]];
        }
	else 
            start--;
    }
    else 
    {
        hash1[start] = challengeConcat[y];
    }  
}