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.
Look up a C++ Reference and learn How To Ask Questions The Smart WayOriginally Posted by Bjarne Stroustrup (2000-10-14)
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
here is the main that i cant changeCode:void encode(char * str){ while (*str != '\0') { str = toupper(str); *str++; } }
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; }PLAINTEXT:
sdahsaghasgjjlksdfgj
sdfds
sdfds
....
?
CIPHERTEXT:
sdahsaghasgjjlksdfgj
sdfds
sdfds
....
?
Last edited by DrowningInCode; 04-26-2010 at 02:48 AM. Reason: forget to show runtime result
Compile your code with a higher warning level and the compiler will help explain the reason to you.
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.
Look up a C++ Reference and learn How To Ask Questions The Smart Way
WOOt done
thanks for the help guys
im beginning to passionately hate CCode:void encode(char * str){ int i= 0; while (*str != '\0') { str[i] = toupper(str[i]); str++; } }
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++; } }
Look up a C++ Reference and learn How To Ask Questions The Smart Way
Why do you have
?Code:str[i] = toupper(str[i]);
What's wrong with:
or even:Code:*str = toupper(*str);
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.
