Thread: Better way to read strings allocating memory?

  1. #1
    Registered User
    Join Date
    Feb 2015
    Posts
    12

    Reading strings allocating memory?

    Why this doesn't work?! It compiles.

    Code:
    void readString(char *s);
    
    int main() {
        char *str;
        readString(str);
        printf("\n %s \n", str);
            
        fflush(stdin);
        getchar();
        
        return 0;
    }
    
    
    void readString(char *s) {
        char *temp;
        
        // allocate space for temp string
        temp = (char *) malloc(100 * sizeof(char));
        
        // read in temp
        scanf("%s", temp);
    
    
        // allocate space in the target string
        s = (char *) malloc( (strlen(temp)+1) * sizeof(char));
    
    
        // memcpy temp to the target string
        memcpy(s, temp, strlen(temp)+1);
    
    
        // free temp
        free(temp);
    }
    Last edited by Gaspar Castillo; 02-11-2015 at 09:31 PM.

  2. #2
    Ultraviolence Connoisseur
    Join Date
    Mar 2004
    Posts
    555
    Several reasons:
    1. You can't call fflush() on stdin that is undefined behavior
    2. You don't limit the size of the string you're reading in so thats a potential buffer overflow (use fgets() instead or limit the scan size of your %s)
    3. When you pass a pointer to a function it's passed by value. This means the original pointer is copied and you get a new pointer that points to the same place as the original one. So clearly you can't change where the original pointer points to within the function, as you are trying to do. For this you have to pass a pointer to the pointer, in order to change where it points to. Alternatively you can also return the allocated memory and assign that to your pointer, but this usually has a higher potential for bugs than the former method.

    Other issues:
    Don't cast the return of malloc, check for errors after calling malloc, scanf, etc.. (read the documentation on your functions)

  3. #3
    Registered User
    Join Date
    Feb 2015
    Posts
    12
    Quote Originally Posted by nonpuz View Post
    Several reasons:
    1. You can't call fflush() on stdin that is undefined behavior
    2. You don't limit the size of the string you're reading in so thats a potential buffer overflow (use fgets() instead or limit the scan size of your %s)
    3. When you pass a pointer to a function it's passed by value. This means the original pointer is copied and you get a new pointer that points to the same place as the original one. So clearly you can't change where the original pointer points to within the function, as you are trying to do. For this you have to pass a pointer to the pointer, in order to change where it points to. Alternatively you can also return the allocated memory and assign that to your pointer, but this usually has a higher potential for bugs than the former method.

    Other issues:
    Don't cast the return of malloc, check for errors after calling malloc, scanf, etc.. (read the documentation on your functions)
    Sorry, but would you help me with some example?

    This is what I've tried to do after reading your feedback, however I haven't got success.

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <strings.h>
    
    
    void readString(char *p);
     
    int main() {
        char *str;
    
    
        // sending the pointer's memory location
        readString(&str);
    
    
        printf("\n %s \n", str);
             
        getchar();
        return 0;
    }
     
     
    void readString(char *p) {
        char *str;
         
        // allocate space for temp string
        str = (char *) malloc(100 * sizeof(char));
         
        // read in temp
        scanf("%100s", str);
    
    
    	// because of "\n" 
        fflush(stdin);
     
        // realloc space in the target string
        str = (char *) realloc(str, (strlen(str)+1) * sizeof(char));
     
     	// pointer's memory location = str's memory location
     	p = str;
    }

  4. #4
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    37,968
    > readString(&str);
    This is good, but you need to fix the prototype and implementation of the function to match.

    Plus, get rid of that fflush(stdin)
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  5. #5
    Registered User
    Join Date
    Feb 2015
    Posts
    12
    Quote Originally Posted by Salem View Post
    > readString(&str);
    This is good, but you need to fix the prototype and implementation of the function to match.

    Plus, get rid of that fflush(stdin)
    Ok, this worked and I understood why,

    Code:
    void readString(char **p);
     
    int main() {
        char *str;
    
    
        // sending the pointer's memory location
        readString(&str);
    
    
        printf("\n %s \n", str);
           
        getchar();
        return 0;
    }
     
     
    void readString(char **p) {
        char *str;
         
        // allocate space for str
        str = (char *) malloc(100 * sizeof(char));
         
        // read in str
        scanf("%100s", str);
     
        // realloc space
        str = (char *) realloc(str, (strlen(str)+1) * sizeof(char));
     
         // pointer's memory location = str's memory location
         *p = str;
    }
    However, if I don't fflush stdin the last getchar() recieves the \n which lives in the buffer after reading "str" and my program exits. What should I do about that?

    Is there any difference, concerning memory, in using a fixed array char str[100] instead of what I am doing?
    Last edited by Gaspar Castillo; 02-12-2015 at 12:44 AM.

  6. #6
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    37,968
    > What should I do about that?
    1. Use fgets()
    2. Read the FAQ on good ways to deal with trailing newlines.

    Also read the FAQ on not casting the return result of malloc.

    > Is there any difference, concerning memory, in using a fixed array char str[100] instead of what I am doing?
    Well that depends on where (and how) you declare the array.
    There is no issue if the array is declared in main, but if you were to declare it in readString, then it would go out of scope when the function returned.

    You could make the array static in readString, but then you couldn't call readString() twice so easily.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  7. #7
    Registered User
    Join Date
    Nov 2012
    Posts
    1,393
    Quote Originally Posted by Gaspar Castillo View Post
    Ok, this worked and I understood why,

    Code:
    void readString(char **p) {
        char *str;
         
        // allocate space for str
        str = (char *) malloc(100 * sizeof(char));
         
        // read in str
        scanf("%100s", str);
     
        // realloc space
        str = (char *) realloc(str, (strlen(str)+1) * sizeof(char));
     
         // pointer's memory location = str's memory location
         *p = str;
    }
    There are two potential issues with this readString function. First, there is a buffer overflow in the line scanf("%100s", str). scanf will read up to 100 characters from stdin into str and then put a '\0' terminator after those 100 bytes (but you currently only allocated 100 bytes). To avoid an overflow you need to allocate one more byte for the '\0' terminator.

    2. If malloc fails it returns NULL, so in the same scanf line you will potentially dereference a NULL pointer. A typical way to handle this is to set the pointer to NULL as soon as an allocation failure happens so that the caller knows an allocation error occured.

    Code:
    void readString(char **p) {
        char *str = malloc(100 + 1); /* 100 characters for str plus '\0' terminator */
        if (!str) {
            *p = NULL; /* failed to allocate memory */
            return;
        }    
        scanf("%100s", str);
        /* ... */
        *p = str;
    }

  8. #8
    Registered User
    Join Date
    Feb 2015
    Posts
    12
    Quote Originally Posted by Salem View Post
    > What should I do about that?
    1. Use fgets()
    2. Read the FAQ on good ways to deal with trailing newlines.

    Also read the FAQ on not casting the return result of malloc.

    > Is there any difference, concerning memory, in using a fixed array char str[100] instead of what I am doing?
    Well that depends on where (and how) you declare the array.
    There is no issue if the array is declared in main, but if you were to declare it in readString, then it would go out of scope when the function returned.

    You could make the array static in readString, but then you couldn't call readString() twice so easily.
    Thanks for your feedback

  9. #9
    Registered User
    Join Date
    Feb 2015
    Posts
    12
    Quote Originally Posted by c99tutorial View Post
    There are two potential issues with this readString function. First, there is a buffer overflow in the line scanf("%100s", str). scanf will read up to 100 characters from stdin into str and then put a '\0' terminator after those 100 bytes (but you currently only allocated 100 bytes). To avoid an overflow you need to allocate one more byte for the '\0' terminator.

    2. If malloc fails it returns NULL, so in the same scanf line you will potentially dereference a NULL pointer. A typical way to handle this is to set the pointer to NULL as soon as an allocation failure happens so that the caller knows an allocation error occured.

    Code:
    void readString(char **p) {
        char *str = malloc(100 + 1); /* 100 characters for str plus '\0' terminator */
        if (!str) {
            *p = NULL; /* failed to allocate memory */
            return;
        }    
        scanf("%100s", str);
        /* ... */
        *p = str;
    }
    Wow, it's easy to forget the '\0' character, thanks.

    I've seen many tutorials in where they always cast malloc(). So, if I don't cast to check errors, I think I should first declare str as void* variable because malloc() is returning a void*

    How do you explain, char *str = malloc(100+1);
    and not void str* = malloc(100+1) ?

  10. #10
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    27,719
    Quote Originally Posted by Gaspar Castillo
    I've seen many tutorials in where they always cast malloc(). So, if I don't cast to check errors, I think I should first declare str as void* variable because malloc() is returning a void*

    How do you explain, char *str = malloc(100+1);
    and not void str* = malloc(100+1) ?
    There is an implicit conversion from void* to any pointer to object such as char*. Therefore, it is neither necessary to cast the return value of malloc nor necessary to declare str as void*.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  11. #11
    Registered User
    Join Date
    Feb 2015
    Posts
    12
    Quote Originally Posted by laserlight View Post
    There is an implicit conversion from void* to any pointer to object such as char*. Therefore, it is neither necessary to cast the return value of malloc nor necessary to declare str as void*.
    Ok.
    Why the hell is casted on a lot of tutorials!? I mean, I'm not saying you're wrong.
    Why casting malloc() bypasses errors?

  12. #12
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    27,719
    Quote Originally Posted by Gaspar Castillo
    Why casting malloc() bypasses errors?
    It is bad practice, but it is possible to call a function without declaring it. When you do so, the compiler considers the parameters and return type of the function to default to int, which can cause problems when that is not actually the case. In the event that you cast the return value of malloc and yet fail to #include <stdlib.h>, you may then suppress the warning that a compiler is likely to raise about the apparent return type of malloc being int, and yet the destination is a pointer.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  13. #13
    Registered User Al3's Avatar
    Join Date
    Nov 2014
    Posts
    135
    As the correct type will be automatically converted. In C++ it could be a bonus in some cases, but not in C.

  14. #14
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    27,719
    Quote Originally Posted by Al3
    In C++ it could be a bonus in some cases, but not in C.
    In C++, there is no implicit conversion from void* to a pointer to object, so the cast is not a "bonus": it is a requirement if one wants to write C++ code that uses malloc, or C code that must be compilable as C++.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  15. #15
    Registered User Al3's Avatar
    Join Date
    Nov 2014
    Posts
    135
    That is what I meant.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Regarding allocating memory
    By monkey_c_monkey in forum C++ Programming
    Replies: 2
    Last Post: 08-29-2012, 10:06 PM
  2. cannot read key when allocating shared memory
    By hjr in forum C Programming
    Replies: 1
    Last Post: 10-19-2009, 11:08 AM
  3. Dynamically allocating memory for strings in a function
    By sunilkjin in forum C Programming
    Replies: 6
    Last Post: 11-22-2006, 04:43 PM
  4. allocating memory?
    By SamuraiDave in forum C Programming
    Replies: 2
    Last Post: 09-21-2005, 02:45 PM
  5. allocating memory screws up data being read in from file
    By Shadow12345 in forum C++ Programming
    Replies: 5
    Last Post: 12-06-2002, 03:23 PM