Thread: Getting string from stdin in C with custom getline(), and getline_s() functions

  1. #1
    Registered User
    Join Date
    Nov 2018
    Posts
    21

    Getting string from stdin in C with custom getline(), and getline_s() functions

    Hi, how can I improve either of these functions?

    Code:
    #include <stdio.h>
    
    
    #ifndef MAX_STR_BUFF_SIZE 
    #define MAX_STR_BUFF_SIZE 300
    #endif // MAX_STR_BUFF_SIZE
    
    
    char* getline(char* _StrOut)
    {
        int ch;
        while ((ch = getchar()) != '\n' && ch != EOF) *_StrOut++ = ch;
        *_StrOut = '\0';
        return _StrOut;
    }
    
    
    char* getline_s(char* _Strout, size_t _Buffer)
    {
        int ch;
        const char* _ptr = _Strout;
        while ((ch = getchar()) != '\n' && (size_t)(_Strout - _ptr) < _Buffer - 1)
        {
            if (ch == EOF) break;
            *_Strout++ = ch;
        }
        *_Strout = '\0';
        return _Strout;
    }
    
    
    int main()
    {
        char input[MAX_STR_BUFF_SIZE];
        getline(input);
        printf("First string: %s\n", input);
        getline_s(input, sizeof(input));
        printf("Second string: %s\n", input);
        return 0;
    }
    Last edited by flash13; 1 Week Ago at 10:49 PM.

  2. #2
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    26,237
    To improve getline, I see three possibilities:
    • Change it to be like POSIX getline, i.e., use dynamic memory allocation. Actually, that there's POSIX getline is a good reason to rename this to say, flash13_getline.
    • Tightly couple flash13_getline with MAX_STR_BUFF_SIZE. I think this is a bad idea.
    • Delete it since you already have getline_s. If you don't plan to use dynamic memory allocation, this is your best option: as it stands, your getline is just another variant of gets, and hence just as inherently vulnerable to buffer overflow.


    For getline_s, I recommend using a library prefix too, e.g., flash13_getline_s. Anyway, instead of computing (size_t)(_Strout - _ptr) < _Buffer - 1) on every iteration, before the loop compute a pointer to one past the last possible non-null character (i.e., a pointer to the last possible null character). You can then just compare _Strout to this pointer. This bounds check should go before the getchar call.

    In both cases, I'm not sure what's the value of returning _Strout. I would have expected a pointer to the start of the resulting string.
    Last edited by laserlight; 1 Week Ago at 02:01 AM.
    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

  3. #3
    Registered User
    Join Date
    Nov 2018
    Posts
    21
    Hi,
    thank you. you are quite cool!.
    I forgot that _StrOut/_Strout was pointing to the last pointer. I've come with something like this for getline():
    Code:
    const char* getline(char* _Strout)
    {
        const char* _ptr = _Strout;
        int ch;
        while ((ch = getchar()) != '\n' && ch != EOF) *_Strout++ = ch;
        *_Strout = '\0';
        return _ptr;
    }
    getline() was meant to be vulnerable to buffer overflow.
    I could solve the problem of getline_s() by returning _ptr and changing the return type to const char*; however, I don't get "instead of computing (size_t)(_Strout - _ptr) < _Buffer - 1) on every iteration, before the loop compute a pointer to one past the last possible non-null character (i.e., a pointer to the last possible null character). You can then just compare _Strout to this pointer. This bounds check should go before the getchar call" , could you show me with an example code? I tried to do what you said, but was getting buffer overflow error. Thanks.
    P.S. when to go for heap allocation and when to remain with stack? I sometimes get dilemma. And what is your thought on alloca()? using this is good or bad practice?

  4. #4
    Registered User rstanley's Avatar
    Join Date
    Jun 2014
    Location
    New York, NY
    Posts
    576
    And what is your thought on alloca()? using this is good or bad practice?
    I would avoid using alloca() for ANY purpose! It allocates memory on the stack, NOT the heap, so one or more allocations large enough could blow the stack! Also, you could NEVER return the address of the allocation, to the calling function, as it would have been freed up!

    Your new version of getline() will not work properly in some cases!!! At the very least, you need to pass the size of the array along with the pointer.

    I would also avoid the use of a leading underscore, even though it is legal. Leading underscores are usually reserved for the compiler, Standard Library, and C Standards use.

  5. #5
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    26,237
    Quote Originally Posted by flash13
    getline() was meant to be vulnerable to buffer overflow.
    I'm sorry, what? Why would you design a function to be deliberately vulnerable to buffer overflow, and then ask how to improve it?

    Quote Originally Posted by flash13
    could you show me with an example code?
    For example
    Code:
    char *laserlight_getline_s(char *buffer, size_t buffer_size)
    {
        char *result = buffer;
        char *end = buffer + buffer_size - 1;
        int ch;
        assert(buffer_size > 0);
    
        while (buffer < end && (ch = getchar()) != '\n' && ch != EOF)
        {
            *buffer++ = ch;
        }
        *buffer = '\0';
    
        return result;
    }
    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

  6. #6
    Registered User
    Join Date
    Nov 2018
    Posts
    21
    hey, thanks. haven't really encountered assert() yet in my learning so far. well, just to see how a flawed function would look like, and why it was getting the buffer overflow.. that's why! and, as for improvement, well I don't know, maybe I thought it was possible to make a better flawed function than what I wrote (doesn't make sense, I know). but I appreciate your feedback.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Need help with strings, pointers and custom functions
    By JorgeChemE in forum C Programming
    Replies: 10
    Last Post: 05-15-2017, 03:44 AM
  2. advise me on custom functions
    By CoCo in forum C Programming
    Replies: 2
    Last Post: 01-21-2011, 08:42 AM
  3. Custom String Class member functions
    By kisiellll in forum C++ Programming
    Replies: 3
    Last Post: 10-03-2009, 09:39 AM
  4. how can I use mutiple getline functions in same app
    By flawedone in forum C++ Programming
    Replies: 2
    Last Post: 01-03-2003, 02:01 PM
  5. Custom gets and puts functions
    By Silverdream in forum C Programming
    Replies: 2
    Last Post: 02-18-2002, 03:12 PM

Tags for this Thread