cuser-input

C - write() repeats after reading user input with spaces


I'm creating a shell program with C and am working on getting user input after displaying the current working directory. I'm using read() and write() as I will need to use signals within my shell later on.

When I test my program, the prompt for input is duplicated whenever I give user input with spaces in it. The number of duplicates created is the number of spaces I give to my user input.

I'm assuming the problem is with my read() call but I'm unsure of how to solve it.

Here is the code:

#include "../include/msgs.h"
#include <linux/limits.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

const char *cwd_err_msg = FORMAT_MSG("getcwd", GETCWD_ERROR_MSG);
const chat *read_err_msg = FORMAT_MSG("read", READ_ERROR_MSG);

char *get_input();

int main() {

  while (1) {
    char cwd[PATH_MAX];

    // error checking
    if (getcwd(cwd, sizeof(cwd)) == NULL) {
      perror(cwd_err_msg);
    }

    strncat(cwd, "$", 1);
    write(STDOUT_FILENO, cwd, strlen(cwd));

    char *input = get_input();

    free(input);
  }

  return 0;
}

char *get_input() {
 char *input = (char *)malloc(100 * sizeof(char));

 if (read(STDIN_FILENO, input, sizeof(input)) == -1) {
   perror(read_err_msg);
 }

 return input;
}

And this is an example of what I see when I test the input:

/home/units/shell/build$test
/home/units/shell/build$testing and testing
/home/units/shell/build$/home/units/shell/build$/home/units/shell/build$

Solution

  • To begin there are a number of problems in the short getinput() function. There are also a number of additional cases you need to take into consideration, to protect against undefined behavior after taking input. A summary of the problems with the current getinput() function (non-exhaustive) are:

    Problems

    Additional Consideration

    As noted in the other comments,

    Improving getinput()

    It doesn't take much to make your getinput() function a bit more robust and useful. If you fix all problems and take into account the listed considerations, you can come up with something like the following. This isn't to say this is the only or best way to solve all issues, but it is a generally robust solution that provides a return indicating success/failure to the caller, passes the buffer to fill as a parameter (and passes a prompt to display if non-NULL). It also validates that the input fits in the MAXINPUT size buffer with the required nul-terminating character and removes the trailing '\n', returning error and emptying the input stream if the input exceeds the storage available.

    #define MAXINPUT    1024      /* if you need a constant, #define one (or more)
                                   * size input buffer as needed, but DON'T SKIMP
                                   */
    
    ssize_t getinput (char *input, const char *prompt)
    {
      ssize_t n = -1;             /* no. of characters read or error */
      int maxinput_exceeded = 0;  /* flag indicating no space for nul-char */
      
      *input = 0;                 /* set input to empty string */
      
      if (prompt != NULL) { /* display prompt if not NULL */
        if ((n = write (STDOUT_FILENO, prompt, strlen (prompt))) == -1) {
          perror ("write-prompt");
          return n;
        }
      }
      
      /* read up to MAXINPUT bytes saving bytes read in n */
      if ((n = read (STDIN_FILENO, input, MAXINPUT)) == -1) {
        perror ("read-input");
        return n;
      }
      
      /* validate space remains to nul-terminate input as C-string or set error
       * flag and empty remaining characters from STDIN_FILENO.
       */
      while (n > 0 && input[n - 1] != '\n' && n == MAXINPUT) {
        maxinput_exceeded = 1;    /* set input exceeded flag */
        n = read (STDIN_FILENO, input, MAXINPUT);
      }
      
      if (maxinput_exceeded) {  /* if input exceeded return error */ 
        write (STDERR_FILENO, "error: input exceeds MAXINPUT\n",
               sizeof "error: input exceeds MAXINPUT\n" -1);
        
        return -1;  
      }
      
      /* nul-terminate input and strip trailing '\n' */
      input[n] = 0;
      input[(n = strcspn (input, "\n"))] = 0;
      
      return n;   /* return number of bytes in resulting string */
    }
    

    Putting together a short example that allows you to test a 10-character buffer by defining TEST10 as part of your compile string, you could do something line the following:

    #include <stdio.h>
    #include <string.h>
    #include <limits.h>
    #include <unistd.h>
    
    #ifdef TEST10
    #define MAXINPUT      10
    #else
    #define MAXINPUT    1024      /* if you need a constant, #define one (or more)
                                   * size input buffer as needed, but DON'T SKIMP
                                   */
    #endif
    
    ssize_t getinput (char *input, const char *prompt)
    {
      ssize_t n = -1;             /* no. of characters read or error */
      int maxinput_exceeded = 0;  /* flag indicating no space for nul-char */
      
      *input = 0;                 /* set input to empty string */
      
      if (prompt != NULL) { /* display prompt if not NULL */
        if ((n = write (STDOUT_FILENO, prompt, strlen (prompt))) == -1) {
          perror ("write-prompt");
          return n;
        }
      }
      
      /* read up to MAXINPUT bytes saving bytes read in n */
      if ((n = read (STDIN_FILENO, input, MAXINPUT)) == -1) {
        perror ("read-input");
        return n;
      }
      
      /* validate space remains to nul-terminate input as C-string or set error
       * flag and empty remaining characters from STDIN_FILENO.
       */
      while (n > 0 && input[n - 1] != '\n' && n == MAXINPUT) {
        maxinput_exceeded = 1;    /* set input exceeded flag */
        n = read (STDIN_FILENO, input, MAXINPUT);
      }
      
      if (maxinput_exceeded) {  /* if input exceeded return error */ 
        write (STDERR_FILENO, "error: input exceeds MAXINPUT\n",
               sizeof "error: input exceeds MAXINPUT\n" -1);
        
        return -1;  
      }
      
      /* nul-terminate input and strip trailing '\n' */
      input[n] = 0;
      input[(n = strcspn (input, "\n"))] = 0;
      
      return n;   /* return number of bytes in resulting string */
    }
    
    int main (void) {
    
      char buf[MAXINPUT] = "";
      ssize_t n = 0;
      
      /* while characters other than '\n' (empty line) read */
      while ((n = getinput (buf, "input: ")) > 0) {
        write (STDOUT_FILENO, buf, n);
        write (STDOUT_FILENO, "\n\n", 2);
      }
      
    }
    

    (note: you simply need to press Enter alone at the input prompt to end the program)

    Example Use/Output

    With a sufficient 1024 byte buffer:

    $ ./bin/readwrite-getinput
    input: my dog has fleas
    my dog has fleas
    
    input: my cat has none
    my cat has none
    
    input: lucky cat :)
    lucky cat :)
    
    input:
    

    Using a compile string for gcc with full warnings enabled and TEST10 defined to test input that exceeds the buffer size, you can compile as follows:

    $ gcc -Wall -Wextra -pedantic -Wshadow -Werror -std=c11 -Ofast -DTEST10 -o readwrite-getinput readwrite-getinput.c
    

    Now exercising the input routine, which provides an error message and exits if input exceeds the storage avaialble:

    $ ./readwrite-getinput
    input: 123456789
    123456789
    
    input: 1234567890
    error: input exceeds MAXINPUT
    

    Look things over and let me know if you have questions.