-1

I want to make a shell where the child process runs linux commands(with the help of execvp) such as "ls" etc. .I also want to be able to run commands with arguments like "ls -a" or "ls -l /tmp" Parent must wait for child to execute the given command with "waitpid". When im trying to run the shell with "ls -a" it takes it as 2 separate commands. Output: ls$-a$

#include "stdio.h"
#include "unistd.h"
#include "stdlib.h"

int main(int argc, char **argv) 
{
    char input[64];
    pid_t pid;
    char *status;
    char *args[64];
    char **next = args;
    char *temp;

    while (1) {
        printf("$");
        fgets(input,"exit",stdin);
        if (strcmp(input, "exit") == 0) {
        exit(0)
        }
        pid = fork();

        if (pid == 0) {
            //child

            *temp = strtok(input, " ");


            while (temp != NULL) {
                *next++ = temp;
                *temp = strtok(NULL, " ");
            }


            if (execvp(args[0], args) == -1) {
                perror("Execvp Error");
            }
            exit(0);

        }else if (pid < 0) {
            printf("error during fork");
        }else {
            //parent
            waitpid(pid, &status, 0);
        }
    }
}
purkavlos
  • 11
  • 3
  • 2
    Great. Did you have a question? – Sean Bright Nov 12 '15 at 22:20
  • you start with `while(strcmp(input, "exit"))` but nothing inited in `input`. After that you read `input` and fork. Even if user enters "exit" (let say). Global logic is not "logical". – hexasoft Nov 12 '15 at 22:35
  • Whenever i run the program i cant see the output of my command...also if put a false command there,its not showing any errors – purkavlos Nov 12 '15 at 22:46
  • There is no point following `scanf("%s", input)` with `*temp = strtok(input, " ")`, since you already know `input` will contain no spaces. – Crowman Nov 12 '15 at 22:51
  • You're passing a `char **` as the second argument to `waitpid()`. That doesn't seem a good idea. – EOF Nov 12 '15 at 22:52
  • Start by taking care about all things people said to you (1. you have a loop that depend on "input" that contains nothing at start 2. you get "input" but fork even if it is "exit" 3. `scanf("%s"…` only read a word, so no space in it 4. waitpid can't work this way). After that you can come back with a more proper code. Note: not seeing that "input" is used uninitialized or "waitpid" used with `char**` means that you compile without warning or don't bother about, in both case it is bad and you should start by understanding these warnings and correct them. – hexasoft Nov 12 '15 at 23:03
  • I edited the code.Thanks for all the answers – purkavlos Nov 12 '15 at 23:23

2 Answers2

1

If you write a few helper functions and come up with a suitable data structure, it becomes trivial. For instance:

main.c:

#define _POSIX_C_SOURCE 200809L

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/wait.h>
#include "stringlist.h"
#include "io.h"


int main(void)
{
    StringList list = NULL;
    get_input(&list);

    while ( strcmp(stringlist_string_at_index(list, 0), "exit") ) {
        pid_t p = fork();

        if ( p < 0 ) {
            perror("fork() error");
            exit(EXIT_FAILURE);
        }
        else if ( p == 0 ) {
            char ** args = stringlist_raw_list(list);
            execvp(args[0], args);
            switch ( errno ) {
                case EACCES:
                    printf("Error: access denied.\n");
                    break;

                case ENOENT:
                    printf("Error: file not found.\n");
                    break;

                default:
                    printf("Error: couldn't fulfill request.\n");
                    break;
            }
            exit(EXIT_FAILURE);
        }
        else {
            int status;
            waitpid(p, &status, 0);
        }

        get_input(&list);
    }

    stringlist_destroy(list);
    return EXIT_SUCCESS;
}

with helper files:

io.h:

#ifndef IO_H
#define IO_H

#include "stringlist.h"

void get_input(StringList * list);

#endif      /*  IO_H  */

io.c:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "io.h"
#include "stringlist.h"


#define MAX_INPUT_LENGTH 256

static void get_input_line(char * buffer, const size_t buffer_size);
static void output_prompt(void);
static StringList tokenize_input(void);


/*  Prompts for and gets input from standard input.
 *
 *  If the StringList pointed to by `list` is not NULL, it will
 *  be destroyed. The StringList pointed to by `list` will be
 *  modified to point to a new StringList created from the input.
 *  If no input is entered, function will prompt for it again.
 *
 *  Note: the input is tokenized purely by space characters, so input
 *  resembling:
 *
 *      cat "Filename with spaces"
 *
 *  will return four tokens, not two. This simple method of tokenizing
 *  will be unsuitable for many applications.
 */

void get_input(StringList * list)
{
    if ( *list ) {
        stringlist_destroy(*list);
    }

    do {
        output_prompt();
        *list = tokenize_input();
    } while ( stringlist_length(*list) == 0 );
}


/*  Gets a line of input from standard input.
 *
 *  Function strips the trailing newline, if present, and
 *  exits the program on error.
 */

static void get_input_line(char * buffer, const size_t buffer_size)
{
    if ( !fgets(buffer, buffer_size, stdin) ) {
        fprintf(stderr, "error getting input\n");
        exit(EXIT_FAILURE);
    }

    const size_t len = strlen(buffer);
    if ( len > 0 && buffer[len - 1] == '\n' ) {
        buffer[len - 1] = 0;
    }
}


/*  Outputs the shell prompt  */

static void output_prompt(void)
{
    printf("shell$ ");
    fflush(stdout);
}


/*  Gets a line of input from standard input and tokenizes it  */

static StringList tokenize_input(void)
{
    StringList list = stringlist_create();

    char input[MAX_INPUT_LENGTH];
    get_input_line(input, MAX_INPUT_LENGTH);

    char * t = strtok(input, " ");
    while ( t ) {
        stringlist_add(list, t);
        t = strtok(NULL, " ");
    }

    return list;
}

stringlist.h:

#ifndef STRING_LIST_H
#define STRING_LIST_H

#include <stddef.h>
#include <stdbool.h>

typedef struct stringlist * StringList;

StringList stringlist_create(void);
bool stringlist_delete_last(StringList list);
void stringlist_destroy(StringList list);
size_t stringlist_length(StringList list);
char * stringlist_string_at_index(StringList list, const size_t index);
char ** stringlist_raw_list(StringList list);
void stringlist_add(StringList list, const char * str);

#endif      /*  STRING_LIST_H  */

stringlist.c:

#define _POSIX_C_SOURCE 200809L

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h>
#include "stringlist.h"

#define DEFAULT_LIST_SIZE 8

struct stringlist {
    char ** list;       /*  Pointer to list of strings      */
    size_t size;        /*  Current capacity of list        */
    size_t top;         /*  Lowest empty element of list    */
};


/*  Creates a new list of default capacity  */

StringList stringlist_create(void)
{
    struct stringlist * new_list = malloc(sizeof *new_list);
    if ( !new_list ) {
        perror("memory allocation failed");
        exit(EXIT_FAILURE);
    }

    new_list->size = DEFAULT_LIST_SIZE;
    new_list->top = 0;

    char ** list = calloc(new_list->size, sizeof *list);
    if ( !list ) {
        perror("memory allocation failed");
        exit(EXIT_FAILURE);
    }

    new_list->list = list;

    return new_list;
}


/*  Deletes the last string in the list.
 *
 *  Returns false if the list was empty, otherwise true.
 */

bool stringlist_delete_last(StringList list)
{
    if ( list->top ) {
        list->top -= 1;
        free(list->list[list->top]);
        list->list[list->top] = NULL;
        return true;
    }
    return false;
}


/*  Destroys the list and frees all resources  */

void stringlist_destroy(StringList list)
{
    while ( stringlist_delete_last(list) ) {
        ;
    }
    free(list->list);
    free(list);
}


/*  Returns the number of strings currently in the list  */

size_t stringlist_length(StringList list)
{
    return list->top;
}


/*  Returns the string at the specified index of the list  */

char * stringlist_string_at_index(StringList list, const size_t index)
{
    return list->list[index];
}


/*  Returns a pointer to the raw list of strings.
 *
 *  This raw list will be NULL-terminated, that is, if the raw
 *  list contains `length` strings, then raw_list[length] == NULL.
 *  This makes the raw list suitable for passing, for instance, to
 *  execv() and friends.
 */

char ** stringlist_raw_list(StringList list)
{
    return list->list;
}


/*  Adds a string to the list.
 *
 *  The raw list will be dynamically resized, if necessary.
 */

void stringlist_add(StringList list, const char * str)
{
    if ( list->top + 1 >= list->size ) {
        char ** new_array = realloc(list->list,
                                    list->size * 2 * sizeof *new_array);
        if ( !new_array ) {
            perror("memory allocation failed");
            exit(EXIT_FAILURE);
        }
        list->list = new_array;
        list->size *= 2;
    }

    char * duped = strdup(str);
    if ( !duped ) {
        perror("memory allocation failed");
        exit(EXIT_FAILURE);
    }

    list->list[list->top] = duped;
    list->top += 1;
    list->list[list->top] = NULL;
}

This adds some error checking for empty input, and responds to failed execvp() calls in a more meaningful way.

Sample session:

Paul@Pauls-iMac:~/Documents/src/sandbox/simple_shell$ ./ss
shell$ 
shell$ 
shell$ ./Fakefile
Error: file not found.
shell$ ./Makefile
Error: access denied.
shell$ ls -alF
total 96
drwxr-xr-x  12 Paul  staff    408 Nov 12 21:18 ./
drwxr-xr-x   6 Paul  staff    204 Nov 12 20:42 ../
-rw-r--r--   1 Paul  staff    368 Nov 12 21:07 Makefile
-rw-r--r--   1 Paul  staff   2016 Nov 12 21:18 io.c
-rw-r--r--   1 Paul  staff    113 Nov 12 21:10 io.h
-rw-r--r--   1 Paul  staff   2240 Nov 12 21:18 io.o
-rw-r--r--   1 Paul  staff   1214 Nov 12 21:08 main.c
-rw-r--r--   1 Paul  staff   1608 Nov 12 21:11 main.o
-rwxr-xr-x   1 Paul  staff  10032 Nov 12 21:18 ss*
-rw-r--r--   1 Paul  staff   2799 Nov 12 20:52 stringlist.c
-rw-r--r--   1 Paul  staff    504 Nov 12 20:53 stringlist.h
-rw-r--r--   1 Paul  staff   2492 Nov 12 21:11 stringlist.o
shell$ ps
  PID TTY           TIME CMD
75221 ttys002    0:00.19 -bash
75678 ttys002    0:00.00 ./ss
shell$ echo Hello, world!
Hello, world!
shell$ cat "Tokenizing input with spaces is generally bad.txt"
cat: "Tokenizing: No such file or directory
cat: input: No such file or directory
cat: with: No such file or directory
cat: spaces: No such file or directory
cat: is: No such file or directory
cat: generally: No such file or directory
cat: bad.txt": No such file or directory
shell$ exit
Paul@Pauls-iMac:~/Documents/src/sandbox/simple_shell$ 
Crowman
  • 25,242
  • 5
  • 48
  • 56
-2

I think that purkavlos here gave us an example code (which is not fully functional yet) in order for us to show him the logic behind running commands through the child process...just correcting parts of the code is not very helping..

Solution: By using gets() instead of scanf() will solve your first problem so that you can accept a string with spaces like Paul said. Check if that works for you cause as i can see tokenization is correct. After that the command should run but im not sure about the output, do you get it via the child?

yakushi
  • 1
  • 1
  • This is a comment, not an answer. And if you want to be "very helping", recommending that someone uses `gets()` is not the way to do it. – Crowman Nov 12 '15 at 23:38
  • Guys pls stop downvoting everybody.This is my first post and im trying to do what you tell me. Anyway still when im trying to run the shell with "ls -a" it takes it as 2 separate commans output: ls$-a$ – purkavlos Nov 12 '15 at 23:45