Like Tree3Likes
  • 3 Post By grumpy

char pointers/app won't work

This is a discussion on char pointers/app won't work within the C Programming forums, part of the General Programming Boards category; Heya, trying to figure out pointers, can't get the following simple app work: all it should do is print the ...

  1. #1
    Registered User
    Join Date
    Nov 2012
    Posts
    38

    char pointers/app won't work

    Heya,

    trying to figure out pointers, can't get the following simple app work:

    all it should do is print the lower letters of a given string

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    
    char* selectLower(char* s);
    
    int main()
    {
        char string[]="This is a TestString";
        printf("%s", selectLower(string));
        return(0);
    }
    
    char* selectLower(char* s)
    {
        char* ptrA=s;
        char* ptrB=malloc(sizeof(char*));
    
        while (*ptrA!='\0')
        {
            if (*ptrA >= 'a' && *ptrA <= 'z')
            {
                *(ptrB++)=*(ptrA++);
            } else {
                ptrA++;
            }
        } *ptrB='\0';
        return ptrB;
    }
    1) why won't it work? :/
    2) is the use of malloc correct at all? I want ptrB to only have the size it really needs (meaning only lower letters). Or do I have to count the amount of lower characters first, store it in some int var and then assign the length via malloc(length)?
    3) can I somehow prevent that my original string s is being overwritten?

    thanks in advance

  2. #2
    Registered User
    Join Date
    May 2009
    Posts
    2,479
    This does NOT allocate enough space.
    Code:
    char* ptrB=malloc(sizeof(char*));
    strlen() +1 should be enough.

    Tim S.
    "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the universe trying to produce bigger and better idiots. So far, the Universe is winning." Rick Cook

  3. #3
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,427
    1) Because your call to malloc is wrong, and you lost a pointer to the start of string you malloc'ed
    2) no
    3) yes

    Here are the problems I found:

    1. You are allocating space for sizeof(char *), which is probably 4 or 8 bytes, depending on whether you have a 32 or 64 bit machine. You need to allocate enough bytes for the entire string, plus one for the null character. You'll use strlen (#include <string.h>).
    2. You keep doing ptrB++ in your loop. So at the end, you do *ptrB = '\0'. Then you return that pointer (pointing to the null terminator) back to the calling function. That makes it look like an empty string.
    3. You have a memory leak, you malloc the memory but never free it. When you fix selectLower, you will be returning a pointer to the start of allocated memory, with only the lower case characters in the string. However, if you don't save that in pointer in a variable, you can't pass it to free later.

  4. #4
    Registered User
    Join Date
    Jun 2005
    Posts
    6,163
    1) To be blunt, it doesn't work because you coded it incorrectly, without understanding the difference between pointers and strings. The selectLower() function returns a pointer to the terminating zero character in the allocated string (or, at least, it attempts to - but the incorrect malloc() call breaks other things). Since printf("%s", anystring) stops writing when it encounters a char with value zero, no output will be produced.

    2) The malloc() call is incorrect. sizeof(char*) yields the size of a pointer. That is a fixed value (albeit compiler dependent) - the value is typically 2 on a 16-bit system, 4 on a 32-bit system, and 8 on 64-bit system. Since the string in main() is longer than 8 characters, your code will overrun that buffer. The length you supply if you want to represent a string needs to be at least the maximum length of the string that will be stored, plus one (for the zero terminator). If you only want the allocated memory to be the number of lowercase letters then, yes, you need to count the lowercase letters. Note: the number of lower-case letters in a string never exceeds the length of that string.

    3) The cause of overwriting your original string is probably the buffer overrun (strictly speaking, anything is allow to happen because of that, but overwriting your original string is one possibility). Correct the problems I've pointed out, making use of the hints I've included, and your problems will go away - unless you introduce another coding problem when doing so.

    4) Your code causes a leak, since the result of a malloc() call is not matched with a free().
    Right 98% of the time, and don't care about the other 3%.

  5. #5
    Registered User
    Join Date
    Nov 2012
    Posts
    38
    thx alot u guys rock it seems to work now!

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    char* selectLower(char* s);
    int count=0;
    
    int main()
    {
        char string[]="This is a TestString";
        printf("%s", selectLower(string));
        return(0);
    }
    
    char* selectLower(char* s)
    {
        char* ptrA=malloc(strlen(s)+1);
    
        while (*s!='\0')
        {
            if (*s >= 'a' && *s <= 'z')
            {
                *(ptrA++)=*(s++);
                count++;
            } else {
                s++;
            }
        } *ptrA='\0';
    
        return ptrA-count;
    }

  6. #6
    Registered User
    Join Date
    Jun 2005
    Posts
    6,163
    Still a memory leak.

    One characteristic of your code is that it is working with arrays, but obscuring that behind pointer syntax. The code can probably made more readable by using array syntax.

    Having expressions with multiple side-effects also makes things less readable - for little or no benefit.

    You might also want to look up the islower() function.
    Right 98% of the time, and don't care about the other 3%.

  7. #7
    Fountain of knowledge.
    Join Date
    May 2006
    Posts
    794
    I don't think the memory 'leak' will be a problem for the OP's code.
    Windows programs release memory on program termination, however it's good practise to release memory when you don't
    need it as it may cause problems in the future if the code is modified.
    However it's going to take a lot of strings to fill up 2gig+ of memory!!!

  8. #8
    Registered User
    Join Date
    Jun 2005
    Posts
    6,163
    Quote Originally Posted by esbo View Post
    I don't think the memory 'leak' will be a problem for the OP's code.
    Windows programs release memory on program termination, however it's good practise to release memory when you don't
    need it as it may cause problems in the future if the code is modified.
    However it's going to take a lot of strings to fill up 2gig+ of memory!!!
    That sort of rubbish (note that I didn't use an expletive, even though I consider it warranted) and lazy attitude is why so many memory leaks occur in production code. It's not a problem, until the same thing is done in a setting where it is a problem. But, having been told it's not a problem, the programmer overlooks it.

    Not all programs run on top of a windows or linux operating system, and not all terminate immediately after leaking so the operating system can clean up. There is probably more code being written for embedded devices (some with and some without any host operating system at all) or for "apps" that never terminate, so their memory leaks are never cleaned up.
    Right 98% of the time, and don't care about the other 3%.

  9. #9
    Registered User
    Join Date
    Nov 2012
    Posts
    38
    Forgot about the leak, thanks!

    Please specify the mentioned concerns:
    One characteristic of your code is that it is working with arrays, but obscuring that behind pointer syntax. The code can probably made more readable by using array syntax.
    Hm did I misuse pointers with char arrays in some way? Or did you just mean that this prog could have been done without pointers too?

    Having expressions with multiple side-effects also makes things less readable - for little or no benefit.
    which ones?

  10. #10
    Registered User
    Join Date
    Jun 2005
    Posts
    6,163
    Quote Originally Posted by coffee_cup View Post
    Hm did I misuse pointers with char arrays in some way? Or did you just mean that this prog could have been done without pointers too?
    By definition, if a program works as intended with pointers, they are not being misused. But, yes, the program could have been done without using pointer syntax. Given a choice of approaches that each produce a required result, it is usually a good idea to pick the one that is more likely to be understood by most people. Bear in mind that more people have difficulty understanding pointers properly than they do understanding arrays).

    Quote Originally Posted by coffee_cup View Post
    which ones?
    There is one assignment, two pointer dereferences, and two side effects in this statement
    Code:
        *(ptrA++)=*(s++);
    Last edited by grumpy; 01-11-2013 at 09:52 PM.
    Right 98% of the time, and don't care about the other 3%.

  11. #11
    Registered User
    Join Date
    Nov 2012
    Posts
    1,051
    Quote Originally Posted by coffee_cup View Post
    Hm did I misuse pointers with char arrays in some way? Or did you just mean that this prog could have been done without pointers too?
    When a function allocates memory, one helpful convention is to decorate the function in some way so that you remember to free it. Here is one example

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    char* newLowString(char* s);
    void delLowString(char* s);
    
    int main()
    {
    	char string[]="This is a TestString";
    	char *t = newLowString(string);
    	printf("%s", t);
    	delLowString(t);
    	return(0);
    }
    
    char* newLowString(char* s)
    {
    	char *retval=malloc(strlen(s)+1);
    	char *ptrA = retval;
    
    	while (*s!='\0')
    	{
    		if (*s >= 'a' && *s <= 'z')
    		{
    			*(ptrA++)=*(s++);
    		} else {
    			s++;
    		}
    	} 
    	*ptrA='\0';
    
    	return retval;
    }
    
    void delLowString(char *s) {
    	free(s);
    }
    Now one can look at the code from main and see that it is correct, without considering the details of what goes on inside of your functions. Whenever you call newLowString, you must eventually call delLowString, and that is a simple rule to follow. You can see a similar idiom in many libraries. For example, if you do fopen(), you should follow it with fclose() later. Don't make excuses like that the OS will do it for you. Be a good boy or girl and clean up after yourself.

  12. #12
    Registered User
    Join Date
    Nov 2012
    Posts
    38
    nice tip! and thanks all for the help! learned alot in this thread

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Second scanf call with char does not work
    By madwizzy in forum C Programming
    Replies: 2
    Last Post: 02-15-2011, 09:37 PM
  2. Replies: 3
    Last Post: 11-17-2008, 11:36 AM
  3. Arrays of Pointers, how do they work?
    By yougene in forum C Programming
    Replies: 10
    Last Post: 11-16-2007, 06:58 PM
  4. char* doesn't work as intended
    By Queue in forum C Programming
    Replies: 10
    Last Post: 09-10-2006, 05:05 AM
  5. Getting my char command to work!!!
    By CumQuaT in forum C++ Programming
    Replies: 4
    Last Post: 09-15-2001, 05:19 AM

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