0

My question is regarding Linux Systems Programming, specifically about the read and write APIs.

I am writing a program that replicates a shell. It takes a string argument and tokenizes it with a space delimiter. Depending on the command of the first token, it performs an operation using the remaining tokens as parameters. So far I have only implemented this for an 'add' command. The code runs in a loop until the user enters 'n' for 'continue? [y/n]'. However, after the first iteration, my program skips the read() after the first write() call to enter command, and ends up at the 'continue?' write() call. Why does it skip the read() call immediately following the first write()?

int main (int argc, char *argv[]) {
int true=0;
while (true==0) {
    char buff1[]="Please enter your command\n";
    int count1= strlen(buff1);
    write (STDOUT_FILENO, buff1, count1);
    char buff2[100];
    int count2=read (STDIN_FILENO, buff2, 100);
    buff2[count2-1]='\0';
    char *list[30]; //This creates an array of character pointers (strings)
    /*
    * Begin tokenization and entering tokens in list
    */
    const char delim[]=" ";
    char *token;
    token=strtok(buff2, delim);
    const char newline[]="\n";
    int i=0;
    while (token!= NULL) {
        write (STDOUT_FILENO, newline, strlen(newline));
        list[i]=token;
        write (STDOUT_FILENO, list[i], strlen(list[i]));
        i++;
        token=strtok(NULL,delim);
    }
    /*
    * End tokenization
    */

    /*
    * Begin Addition operation
    */
    const char add[]="add";
    if (strcmp(list[0], add)==0) {
        int result=0;
        for (int j=1; j<i; j++) {
            result+=atoi(list[j]);
        }
        char sum[50];
        int sumcount=sprintf(sum, "%d", result);
        write (STDOUT_FILENO, newline, strlen(newline));
        write (STDOUT_FILENO, sum, sumcount);
    }
    /*
    * End Addition operation
    */


    char *truefalse;
    char endmessage[]="Continue: [y/n]\n";
    write (STDOUT_FILENO, endmessage, strlen(endmessage));
    read (STDIN_FILENO, truefalse, 1);
    if (*truefalse=='n') {
        true=1;
    }

}
return 0;
}

As this output image shows, in the second iteration, after asking me to enter a command, the code skips to asking me to continue rather than actually reading my command

  • for ease of readability and understanding: 1) consistently indent the code. indent after every opening brace '{'. unindent before every closing brace '};'. Suggest each indent level be 4 spaces. – user3629249 Mar 13 '18 at 17:31
  • the posted code does not compile!. it is missing the needed `#include` statements for the header file(s) that expose the prototypes for the functions called from the C library. Such as `stdio.h`, `string.h` and `unistd.h` – user3629249 Mar 13 '18 at 17:33
  • regarding: `char *truefalse;` and `read (STDIN_FILENO, truefalse, 1);` the pointer `truefalse` is never set to point to some memory that the application owns. It will contain what ever trash is on the stack as its' location. Setting a value where that pointer points is (therefore) undefined behavior and can lead to a seg fault event. – user3629249 Mar 13 '18 at 17:39
  • the returned value from a call to `read()` should be checked to assure the operation was successful. – user3629249 Mar 13 '18 at 17:40
  • regarding: `int main (int argc, char *argv[])` the parameters `argc` and `argv` are not used, This causes the compiler to output 2 warnings messages. Suggest using the signature: `int main( void )` – user3629249 Mar 13 '18 at 17:43
  • the function: `strlen()` returns a `size_t`, which is (usually) a `unsigned long int` assigning a `unsigned long` to a `int` is problematic. Suggest declaring `count1` as `size_t` rather than `int`. And because `read()` returns a `ssize_t`, suggest declaring `count2` as `ssize_t` rather than `int` Note: `read()` can return 0 (means EOF) or <0 (means system error occurred.) The code needs to check for these conditions – user3629249 Mar 13 '18 at 17:47
  • You’re right. I forgot to add the include statements. This was really helpful. Thanks ! – Omer Farooq Ahmed Mar 13 '18 at 17:49

1 Answers1

1

Your program has undefined behavior.

You use a pointer that has not been initialized to point to anything valid.

The line

char *truefalse;

declares a pointer but it has not been initialized to point to anything valid. You go on to use it in the line

read (STDIN_FILENO, truefalse, 1);

Instead of

char *truefalse;
char endmessage[]="Continue: [y/n]\n";
write (STDOUT_FILENO, endmessage, strlen(endmessage));
read (STDIN_FILENO, truefalse, 1);

Use

char truefalse; // Make it an object instead of a pointer.
char endmessage[]="Continue: [y/n]\n";
write (STDOUT_FILENO, endmessage, strlen(endmessage));
read (STDIN_FILENO, &truefalse, 1); // Use the address of the variable.

Update

The reason your code does not wait for you to enter anything in the second iteration is that the newline character is still left in the input stream. The second call just reads the newline character.

You'll need code to skip the rest of the line after reading the answer to the question.

The simplest way would be to use:

int c;
while ((c = fgetc(stdin)) != EOF && c != '\n');
Community
  • 1
  • 1
R Sahu
  • 204,454
  • 14
  • 159
  • 270