Thread: working with srings adresses and pointers,

Originally Posted by Lired

So the code i posted before, this one:
<snip>
is still wrong?? Doesn't it correct the situation in wich they overlap?

It's still wrong. You can subtract pointers of the same type, but

Originally Posted by C11 6.5.6 p9

9 When two pointers are subtracted, both shall point to elements of the same array object,
or one past the last element of the array object;

If they don't point to the same array object (which is possible), your code will exhibit undefined behavior. I think the only practical solution here is to copy everything into a temp array.

A few notes:
You should not limit yourself to 1000 for a temp array. Also, it would be a waste if there were only 3 bytes to copy. You can use VLAs to make a temp array of the right size (see below), but if that size if very large (e.g. ~1MB), you may run into issues. A ~1MB+ copy is probably not an issue for this assignment, but I can't say for sure.

Code:

void movefunction(char m1[], char m2[], int n) {
    char temp[n];

Can you please give me an example with wich my function doesnt work so that i can understand better? Otherwise i cant fix something that i dont understand what is wrong to begin with.

You are doing m1-m2. That is subtraction of two pointers (m1 and m2 are pointers to char despite the [] syntax). Subtraction of two pointers is only allowed if they both point to somewhere in the same array*. If they are not of the same array object, your code exhibits undefined behavior.

Code:

char foo[100] = "some stuff";
char bar[100] = "other stuff";
movefunction(foo, bar, 7);

That will cause your code to break. foo and bar point to different array objects, resulting in undefined behavior when you subtract one from the other.

* That is an over simplification. Read the C standard, section 6.5.6 in full for more detail and the "exceptions".

So if my code is like this:

Code:

void movefunction (char m1[], char m2[], int n){
    char temp[10000];
    int i;

    if((((sizeof(m1))-(sizeof(m2)))<0)||((sizeof(m1)-sizeof(m2))<(sizeof(n)))){
        for (i=0;i!=n;i++){
                temp[i] = m2[i];
            }
        for (i=0;i!=n;i++){
                m1[i] = temp[i];
            }
    }
    else {
        for (i=0;i!=n;i++){
                   m1[i] = m2[i];
            }
    }
}

it is correct already right? Since now it isn't the pointers that are being subtracted but their size with size_t as their type. There shouldn't be any problem now unless I am understanding this wrong, wich is possible, since I'm still learning and many things that you said I'm not sure about them.

Originally Posted by Lired

So if my code is like this:
...
it is correct already right? Since now it isn't the pointers that are being subtracted but their size with size_t as their type. There shouldn't be any problem now unless I am understanding this wrong, wich is possible, since I'm still learning and many things that you said I'm not sure about them.

No, sizeof is not doing what you think. It will always be the same for m1 and m2 so long as they have the same type. Read this: Question 6.21. Besides, even if it did what you thought it was doing, it would only tell you which one was bigger, not whether they have any overlap. Furthermore, what did you intend by sizeof(n). n is the number of bytes being copied. sizeof(n) is the size of an int -- likely 4 bytes if you're on a 32-bit system, and 8 bytes on a 64.

Simply use a temp array, just make it have n elements instead of, say, 1000. Don't worry about overlap and copy direction since you're not accessing the source and dest at the same time.

So you are telling me to do something like this:

Code:

void movefunction (char m1[], char m2[], int n){
    char temp[n];
    int i;
    int r;
    for(r=0;m2[r]!=m2[n];r++){
            temp[r]=m2[r];
    }
    temp[n+1]='\0';
    for (i=0;i!=n;i++){
               m1[i] = temp[i];
        }
}

or something like this:

Code:

void movefunction (char m1[], char m2[], int n){
    char temp[1000];
    int i;
    int r;
    for(r=0;m2[r]!=m2[n];r++){
           temp[r]=m2[r];
    }
    temp[n+1]='\0';
    for (i=0;i!=n;i++){
               m1[i] = temp[i];
        }
}

am i right this time? And it makes sense, if m1 and m2 shared the same memory space before, i use temp wich wont share it.

It seems like you're just writing random code and posting it here, without putting serious thought into it. Maybe I'm wrong, but that's the impression I'm getting. You're certainly not bothering to try it out for yourself since the code you posted is definitely broken.

The first one has the right declaration of temp

Code:

char temp[n];

But both have incorrect loops for copying into temp:

Code:

for(r=0;m2[r]!=m2[n];r++){

Think carefully about what your loop condition (the middle part) is. What would happen if the data were something like "aabbaa"? What if there are no repeats in the data? Also, accessing m2[n] may be out of bounds.

Both also have a problem here:

Code:

temp[n+1]='\0';

Again, you have an out of bounds error. An array of n elements has valid indices of 0..n-1. Why are you trying to null terminate temp?

Also, you don't need two separate index variables. Get rid of r and reuse i.

So i got rid of the unnecessary r. I corrected the loop condition, it now copies untill string m2 ends (m2=='\0'), but since temp is an array with n elements it only copies the first n elements so there is no need for temp[n]='\0'.

I tested this new version and with my tests it seems to be okay.

for example with face, hours and 3 it returns houe.

Code:

void movefunction (char m1[], char m2[], int n){
    char temp[n];
    int i;
    for(i=0;m2[i]!='\0';i++){
            temp[i]=m2[i];
    }
    for (i=0;i!=n;i++){
               m1[i] = temp[i];
        }
}

Test more thoroughly. What if the source string is "The quick brown fox jumps over the lazy dogs." and you ask it to copy 3 bytes?

Also, the exercise doesn't say anything about strings. It seems you were the one that brought strings into this, so you need to decide what is to be done. Do you always copy n bytes, even if the '\0' is somewhere before the nth byte? Do you null terminate the result? Part of the reason this is difficult is because you don't have a clear idea of what the problem is asking. In my understanding, it is as follows:
It must move n bytes from m2 to m1, regardless of whether those bytes are strings, have a null terminator, etc. It should be able to do so even if m1 and m2 overlap.

Originally Posted by anduril462

Test more thoroughly. What if the source string is "The quick brown fox jumps over the lazy dogs." and you ask it to copy 3 bytes?

Also, the exercise doesn't say anything about strings. It seems you were the one that brought strings into this, so you need to decide what is to be done. Do you always copy n bytes, even if the '\0' is somewhere before the nth byte? Do you null terminate the result? Part of the reason this is difficult is because you don't have a clear idea of what the problem is asking. In my understanding, it is as follows:
It must move n bytes from m2 to m1, regardless of whether those bytes are strings, have a null terminator, etc. It should be able to do so even if m1 and m2 overlap.

Well the exercise is together with others that are about string manipulation so i know for a fact it is about strings, i also know for a fact that the string is a single word, not separated by spaces.

Well now i see, when the source string is big my function doesnt work... The question is how big can it be? And why doesnt it work if it is big?

And yes i also think it must move n bytes regardeless there is a null terminator, so, if i use this example:
hours
yes
4
it should return yes.

So having this into consideration, am i going on the right direction with my previous code or should i just scrap it and start anew? Because i cant seem to understand what is the problem that makes my program crash when the source is big...

and what does even mean that they overlap? I mean, even if they share memory, saying that m1[1]=m2[1] for example shouldn't be a problem since it would just stay the same value that it was already, the end result would be the same.

So in what cases they overlap? isnt it when they have the same memory adress? then does it mean the strings are equal, like hello is equal to hello? or is it some deeper meaning that i don't understand?

How do you know for a fact a string does not include spaces? In C, a string does not stop at white space. The scanf family of functions behaves that way with the %s modifier, but that is not the general meaning of a string in C. A string in C is a sequence of adjacent characters (including white space) that ends with a zero byte (a.k.a. null character or '\0').


Also, you should consider that, while there may have been other exercises about strings, this does not seem like a string exercise. The requirements sound exactly like that of the standard C function memmove(), which is not exactly a string despite not being in the string.h. It's there because arrays of bytes that aren't strings are very similar to arrays of bytes which are strings.


You state it should move n bytes regardless of the null terminator. Yet your example is not very good since the null terminator happens to be the 4th byte.


As for overlap: that means the area of memory you are copying from shares some bytes with the area of memory you are copying to. But that doesn't mean they're equal. Imagine i had a 100 byte array. I want to copy the last 99 bytes into the first 99 spots (i.e. "slide everything down 1"). They are not equal, but they overlap. If done incorrectly, your move function could overwrite part of the source (since the destination might overlap there) before it has been read/copied from. I should be able to do

Code:

char foo[] = "abcdefghij";
movefunction(&foo[4], &foo[3], 3);

And get the correct string "abcddefhij". If the overlap is handled incorrectly, a common mistake would result in the string "abcefgghij".


Don't scrap your code and start anew -- you're fairly close. Simply put the keyboard away, and get out the paper and pencil. Graph paper would be good. Write a big long string (with or without spaces), one letter per square. In the last square, put a \0 to signify the null terminator. Write out several sample ways to call your move function. Your examples should include every possible combination of how your function can be called: source and destination and length create overlap or don't; source comes before dest or after, or they're the same; there is a null terminator in the source or there isn't; the null terminator is less than n chars into the source, or exactly n chars. Use other groups of squares to represent temp, and possibly a different destination array if you want. Work through those example by hand until you understand exactly what your code should be doing. Once you truly understand the algorithm, then writing the code is fairly simple.

so, if i understood correctly, this code:

Code:

void movefunction (char m1[], char m2[], int n){
    char temp[1000];
    int i;
 
    if(((m1-m2)<0)||((m1-m2)<n)){
        for (i=0;i!=n;i++){
                temp[i] = m2[i];
            }
        for (i=0;i!=n;i++){
                m1[i] = temp[i];
            }
    }
    else {
        for (i=0;i!=n;i++){
                   m1[i] = m2[i];
            }
    }
}

only doesn't work if the arrays are not of the same type? But since they are both char arrays then the pointers can be subtracted, right? So for strings this code is correct and it even works with your example (abcdefghij, it returns abcddefhij)


also with my other code:

Code:

void movefunction (char m1[], char m2[], int n){
    char temp[n];
    int i;
    for(i=0;m2[i]!='\0';i++){
            temp[i]=m2[i];
    }
    for (i=0;i!=n;i++){
               m1[i] = temp[i];
        }
}

that example is also correct here, the only problem seems to be when the source is too big (that problem doesnt exist on the previous code). Also when i compile this with gcc -ansi -Wall -pedantic -O2 testar.c it gives the following warning:

testar.c:49:5: warning: ISO C90 forbids variable length array ‘temp’ [-Wvla]
char temp[n];
^

so iguess they don't want me to use variables as the length of the string.