Simple method terrible implementation

This is a discussion on Simple method terrible implementation within the C Programming forums, part of the General Programming Boards category; ok so i am new to C and pointers, that being said please forgive me for what must be really ...

  1. #1
    Registered User
    Join Date
    Mar 2010
    Posts
    4

    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;
    }

  2. #2
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    22,304
    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.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  3. #3
    Registered User
    Join Date
    Jan 2009
    Location
    Australia
    Posts
    375
    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!

  4. #4
    Registered User
    Join Date
    Mar 2010
    Posts
    4
    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;
    }
    PLAINTEXT:
    sdahsaghasgjjlksdfgj
    sdfds

    sdfds
    ....
    ?

    CIPHERTEXT:
    sdahsaghasgjjlksdfgj
    sdfds

    sdfds
    ....
    ?
    Last edited by DrowningInCode; 04-26-2010 at 03:48 AM. Reason: forget to show runtime result

  5. #5
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    22,304
    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.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  6. #6
    Registered User
    Join Date
    Mar 2010
    Posts
    4
    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

  7. #7
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    22,304
    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++;
        }
    }
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  8. #8
    Registered User
    Join Date
    Oct 2008
    Posts
    1,262
    Quote Originally Posted by DrowningInCode View Post
    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]);

  9. #9
    Registered User
    Join Date
    Mar 2010
    Posts
    4
    Quote Originally Posted by EVOEx View Post
    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.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. DirectX | Drawing text
    By gavra in forum Game Programming
    Replies: 4
    Last Post: 06-08-2009, 01:23 AM
  2. Simple C# structure question
    By sketch in forum C# Programming
    Replies: 4
    Last Post: 09-14-2007, 05:29 PM
  3. Replies: 6
    Last Post: 08-29-2007, 09:52 PM
  4. Simple method of coloured text?
    By SPiRiToFCaT in forum C++ Programming
    Replies: 3
    Last Post: 09-04-2002, 10:46 PM
  5. Replies: 3
    Last Post: 12-03-2001, 01:45 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21