Custom user input function

Question

The program consists of 3 files: demo.c, mylib.c and mylib.h. The code is heavily commented.

demo.c:

/***********************************************************************
 *
 * This is a program that prompts the user for an input string using a
 * custom input function.
 *
 ***********************************************************************/
#include <stdio.h>  /* printf */
#include <stdlib.h> /* exit */
#include "mylib.h"


/* For testing purposes, let's make the length of the string 4 */
#define STRING_LENGTH 4



int main(int argc, char* argv[]) {
    char* s = udf_get_input("Prompt: ", STRING_LENGTH);
    printf("You entered: \"%s\"\n", s);
    /* I put quotes around the string to better see
       what has actually been entered */

    free(s);
    s = NULL; /* This is not really necessary */


    exit(EXIT_SUCCESS);
}

mylib.h:

#ifndef MYLIB_H_
#define MYLIB_H_


size_t udf_strlen(const char* s);
char*  udf_get_input(const char* const prmpt, int str_len);


#endif /* MYLIB_H_ */

mylib.c:

#include <stdio.h>  /* printf, getchar */
#include <stdlib.h> /* malloc, realloc, free, exit */


#define ERROR_MSG "error: could not allocate enough memory"



/***********************************************************************
 *
 * This is just my own implementation of the C Standard Library's
 * strlen function. We're going to need it later.
 *
 ***********************************************************************/
size_t udf_strlen(const char* s) {
    size_t i = 0;

    while (*s++) {
        i++;
    }

    return i;
}


/***********************************************************************
 *
 * This is a function that takes in as arguments a pointer to a string
 * that represents the prompt the user sees when typing things at the
 * keyboard and the length of the string. The function returns a pointer
 * to the string that has been entered.
 *
 * How it works:
 * We are going to allocate a certain number of bytes on the heap. This
 * is going to be our buffer. Then we will read whatever the user types
 * in into that buffer. After that, we will check if we need to tweak
 * the amount of memory that the string takes up so that no memory is
 * wasted unnecessarily. If the number of characters entered by the user
 * exceeds the buffer size, the rest of the string is discarded.
 *
 ***********************************************************************/
char* udf_get_input(const char* const prmpt, int str_len) {
    int   buffer_size = str_len + 1; /* Number of characters allowed to be
                                        entered plus one to accommodate
                                        the null character */
    char* buffer;            /* Temporary storage for the user's string  */

    if (!(buffer = malloc(buffer_size * sizeof(char)))) {
        printf("%s\n", ERROR_MSG);
        exit(EXIT_FAILURE);
    }

    printf("%s", prmpt);     /* Display the prompt                       */
    int ch;                  /* Stores characters retrieved from stdin   */
    char* p = buffer;        /* Temporary pointer to traverse the buffer */
    while ((ch = getc(stdin)) != EOF) {
        /* If the character read is a newline character or buffer_size - 1
           characters have been already entered, terminate the string with a
           null character and bail out of the loop */
        if (ch == '\n' || !--buffer_size) {
            *p = '\0';
            break;
        }
        *p++ = ch;
    }

    /* If buffer_size is more than zero, that means there are unused bytes in
       the buffer. So, we will reallocate memory to shirk it so that the string
       occupies as much memory as exactly necessary. Otherwise no memory
       reallocation is needed and we can skip this step. */
    if (buffer_size) {
        buffer = realloc(buffer, (udf_strlen(buffer) + 1) * sizeof(char));
        if (!buffer) {
            printf("%s\n", ERROR_MSG);
            exit(EXIT_FAILURE);
        }
    }

    return buffer; /* Return the pointer to the string stored on the heap */
}

---

To test the program, make a separate directory and create these three files in it:

touch demo.c mylib.c mylib.h

To run the program, execute this command:

gcc -c demo.c mylib.c && \
gcc demo.o mylib.o -o a.out && \
./a.out

Answer 1

In general, the library seems well designed to me (correctly split into header and implementation, together with inclusion guards). I have some smaller suggestions for improvements.

Size of input string: (See also the answer of @ Frederik Deweerdt) instead of int, I recommend using size_t also for the length of the string in udf_get_input (like you did for the return value of udf_strlen). In this way, you avoid getting negative values for the length. Also, malloc expects size_t.

Error handling: it is very good that you check for success of malloc/realloc. However, I would leave the consequence (print message/exit from the program) up to the client of your library, instead of doing it yourself. I suggest that your function returns NULL on failure, and an empty string (i.e. "\0") if the user entered nothing. In this way, the two cases can be differentiated. If you want to return the reason of the failure (i.e., malloc failed due to no memory), there are various options:

• Check how malloc reports failures on your system, e.g. does it set errno? If so, you do not need to do anything else, besides returning NULL. The client can check errno, if they want to.

• Instead of the string that the user typed, return an integer error code (which you appropriately set), and pass the string as a pointer to pointer:

.

int udf_get_input(const char* const prmpt, size_t str_len, char ** user_input) {
    /* ... */
    *user_input = buffer;
    /* ... */
}

Note, that if you do not exit from the program (as I suggest), you may need to clean up allocated resources when you return. (E.g., if the first malloc was successful, but then realloc fails.) You can find hints on how to properly clean up e.g. here and here.

Commenting: Usually, I recommend to write self-explaining code (with good naming), and use comments sparingly, only when something is not really clear from the code. I am not aware of any hard rule, but in your code there are parts which are over-commented, i.e., comments are not necessary at all (in my opinion), or parts which could be made more self-explanatory with name-changes (and hence comments be avoided). Some concrete examples:

/* For testing purposes, let's make the length of the string 4 */

I suggest removing this comment, or changing it to something more concise like "maximal length of the input string". Even better, just rename the constant to something like MAX_INPUT_LENGTH, and then you can leave off the comment.

/* I put quotes around the string to better see
   what has actually been entered */

I suggest removing this comment completely (as what it states is obvious to the reader of the code in my opinion).

Descriptions of the functions: I suggest moving them to the header files, i.e. the parts that describe what those functions are doing (the whole description for udf_strlen and the first paragraph of udf_get_input. (The "How it works" part can remain in the implementation, as I think that is not interesting for the user of the function.)

I also suggest leaving out (at least) the following two comments, as they are stating the obvious:

/* Display the prompt */

/* Return the pointer to the string stored on the heap */

Answer 2

The code looks very good overall.

A couple of nitpicks:

• Why not use size_t for str_len?
• sizeof(char) is guaranteed to be 1, not using it in the malloc call would be more concise.

Then, when if getc does return EOF, it looks like buffer won't be null-terminated, will it?

Answer 3

the following code:

1. corrects the warnings from the compiler
2. corrects the logic errors
3. removes unnecessary clutter from the code

and now, the code

// myLib.h
#ifndef MYLIB_H_
#define MYLIB_H_

//added
#include <stddef.h>  // size_t

size_t udf_strlen(const char* s);
char*  udf_get_input(const char* const prmpt, size_t str_len);


#endif /* MYLIB_H_ */

// myLib.c
#include <stdio.h>  /* printf, getchar, perror */
#include <stdlib.h> /* malloc, realloc, free, exit */


#define ERROR_MSG "error: could not allocate enough memory"



/***********************************************************************
 *
 * This is just my own implementation of the C Standard Library's
 * strlen function. We're going to need it later.
 *
 ***********************************************************************/
size_t udf_strlen(const char* s)
{
    size_t i = 0;

    while (*s++) {
        i++;
    }

    return i;
}


/***********************************************************************
 *
 * This is a function that takes in as arguments a pointer to a string
 * that represents the prompt the user sees when typing things at the
 * keyboard and the length of the string. The function returns a pointer
 * to the string that has been entered.
 *
 * How it works:
 * We are going to allocate a certain number of bytes on the heap. This
 * is going to be our buffer. Then we will read whatever the user types
 * in into that buffer. After that, we will check if we need to tweak
 * the amount of memory that the string takes up so that no memory is
 * wasted unnecessarily. If the number of characters entered by the user
 * exceeds the buffer size, the rest of the string is discarded.
 *
 ***********************************************************************/
char* udf_get_input(const char* const prmpt, size_t str_len) {
    size_t   buffer_size = str_len + 1; /* Number of characters allowed to be
                                        entered plus one to accommodate
                                        the null character */
    char* buffer = NULL;            /* Temporary storage for the user's string  */

    if (!(buffer = malloc(buffer_size)))
    { // then, malloc failed
        perror( "malloc failed:" );
        printf("%s\n", ERROR_MSG);
        exit(EXIT_FAILURE);
    }

    printf("%s", prmpt);     /* Display the prompt                       */
    int ch;                  /* Stores characters retrieved from stdin   */
    char* p = buffer;        /* Temporary pointer to traverse the buffer */
    while ( buffer_size >0 && (ch = getc(stdin)) != EOF && '\n' != ch)
    {
        *p++ = (char)ch;
        buffer_size--;
    }

    *p = '\0';

    return buffer; /* Return the pointer to the string stored on the heap */
}

 /***********************************************************************
 *
 * This is a program that prompts the user for an input string using a
 * custom input function.
 *
 ***********************************************************************/
#include <stdio.h>  /* printf */
#include <stdlib.h> /* exit */
//#include "mylib.h" -- contents posted above


/* For testing purposes, let's make the length of the string 4 */
#define STRING_LENGTH 4



int main( void ) 
{
    char* s = udf_get_input("Prompt: ", STRING_LENGTH);

    printf("You entered: \"%s\"\n", s);
    /* I put quotes around the string to better see
       what has actually been entered */

    free(s);
    s = NULL; /* This is not really necessary */

}

Answer 4

Simpler implementation of strlen(). No need to increment 2 variables.

size_t udf_strlen(const char* s) {
    const char* p = s;

    while (*p) {
     p++;
    }

    return (size_t) (p-s);
}

Functionality

1) Incorrect functionality when a few characters read followed by EOF and not '\n' as it leaves buffer[] without a null character termination.

2) Better to not consume ch from stdin if no more room available. Do not read or put ch back with ungetc().

3) Return NULL when nothing read.