Simple method terrible implementation

ok so i am new to C and pointers, that being said please forgive me for what must be really crappy code.

i want to rewrite a string so that all the characters in it become upper case
this is the method im having trouble with; when my code reaches it, it gets a segmentation fault (NOoOOooOoO)

Code:

---

void    encode(char * str){
        char charAt;
        char *temp = malloc (1000*sizeof(char));
        int i=0;
        while (*str != EOF)
                {       
                        charAt = *str;
                        charAt = toupper(charAt);
                        temp[i]=charAt;
                        i++;
                        str++;
                }
                *str = temp;
}

---

You should compare with '\0', not EOF. But there's more: you cannot just malloc(1000), because the string may have a length greater than or equal to 1000.

I suggest that you do not use malloc at all: write the function such that the changes the original string.

EOF signifies the end of data for a file, it's usually -1.

The end of a string is denoted by an element of value 0, represented in ASCII as '\0'.

Also *str = temp is going to set the last element of str to the value temp, which is a pointer to some data. This will knock out the '\0' and could cause a segmentation fault later on. If you wish to return your new array to the calling function, then you can change the function type to char* and add 'return temp' to the end of your function.

This could also be done without the use of 'temp', by just changing the parameter being passed in, it would be much easier but the fact that you are doing it like this suggests that this is the required way, or you wish to experiment with dynamic memory allocation.

Remember to free 'temp' in the calling function!

ok simplified it
works now but it does not change the text for some reason

Code:

---

void    encode(char * str){
        while (*str != '\0')
                {       
                        str = toupper(str);
                        *str++;
                }
}

---

here is the main that i cant change

Code:

---

int main(int argc, char **argv) {
        char *text;
        if (argc != 2) {
                printf("USAGE: %s filename\n", argv[0]);
                exit(1);
        }
        text=read_text(argv[1]);
        printf("PLAINTEXT:\n%s\n", text);
        encode(text);
        printf("CIPHERTEXT:\n%s\n", text);
 /*     
                decode(text);
        printf("DECODED CIPHERTEXT:\n%s\n", text);
*/
        free(text);
        return 0;
}

---

Quote:

---

PLAINTEXT:
sdahsaghasgjjlksdfgj
sdfds

sdfds
....
?

CIPHERTEXT:
sdahsaghasgjjlksdfgj
sdfds

sdfds
....
?

---

Quote:

---

Originally Posted by DrowningInCode

works now but it does not change the text for some reason

---

Compile your code with a higher warning level and the compiler will help explain the reason to you.

Quote:

---

Originally Posted by DrowningInCode

here is the main that i cant change

---

Right. It also means that you should not be dynamically allocating memory in the encode function. It is the read_text function that should do that job.

WOOt done
thanks for the help guys

Code:

---

void    encode(char * str){
        int i= 0;
        while (*str != '\0')
                {       
                        str[i] =  toupper(str[i]);
                        str++;
                }
}

---

im beginning to passionately hate C

Well, that is correct, but the variable i is actually unnecessary. It would be simpler to write:

Code:

---

void encode(char *str) {
    while (*str != '\0')
    {
        *str = toupper(*str);
        str++;
    }
}

---

Quote:

---

Originally Posted by DrowningInCode

WOOt done
thanks for the help guys

Code:

---

void    encode(char * str){
        int i= 0;
        while (*str != '\0')
                {       
                        str[i] =  toupper(str[i]);
                        str++;
                }
}

---

im beginning to passionately hate C

---

Why do you have

Code:

---

                        str[i] =  toupper(str[i]);

---

?

What's wrong with:

Code:

---

                        *str =  toupper(*str);

---

or even:

Code:

---

                        str[0] =  toupper(str[0]);

---

Quote:

---

Originally Posted by EVOEx

Why do you have

Code:

---

                        str[i] =  toupper(str[i]);

---

?

What's wrong with:

Code:

---

                        *str =  toupper(*str);

---

or even:

Code:

---

                        str[0] =  toupper(str[0]);

---

---

well sir i have neither rhyme nor reason for doing what i do :-P
thank you for bringing the alternates back into my mind.