C shell with background tasks

Question

I'm about to implement a shell in C and I've got some questions.

It should be able to run programms in the foreground or in the background. If you enter "job" before a command, it should be run in the background and I want to organize these background tasks in a list. With the command "list" all background tasks should be displayed on the command line.

If I add some background tasks with the job-command, I have got the problem that the pid is always 0, for example when I add two tasks and then do the list-command, both have the pid 0. What is my mistake here?

Another problem is, that I want to set the attribute "command" for the new list element, but always the command for every element of the list will be set to this value. Where am I wrong here? How can I fix this issue?

Here's my code:

shell.c (the main class):

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <unistd.h>
#include <errno.h>

#include "dll.h"

#define MAXIMUM_INPUT 256

struct proc_info {
    struct list_head head;
    pid_t pid;
    char* command;
    char* running;
    int number;
    int status;
};

void print_procs(struct proc_info *proc_list);

void executecmd(char* command);

int makeargv(const char *s, const char *delimiters, char ***argvp);

void freemakeargv(char **argv);

int main() {
    //Initialize list
    struct list_head anchor;
    list_init(&anchor);
    int count = 0;
    char input[256];
    pid_t newProcess;

    while(1) {

        //Prompt
        printf("tsh> ");

        //Read input
        if (fgets(input, 256, stdin) == NULL)
            continue;
        int len = strlen(input);
        if(len == 1) //Only the "\n", fgets puts "\n" at the end of the string
            continue;
        else if (input[len - 1] == '\n')
            input[len - 1] = 0;

        //Make a list of strings with each argument out of the string of all arguments
        char** args;
        makeargv(input , " ", &args);

        char* firstArgument = *args;

        if(strcmp(firstArgument, "job") == 0) {
            char *command;
            strncpy(command, input+4, len-4);

            struct proc_info *new_proc;
            new_proc = malloc(sizeof(struct proc_info));
            new_proc->command = command;
            new_proc->number = count;
            new_proc->running = "running";
            count++;
            list_add_tail(&(new_proc->head), &anchor);

            if ((newProcess = fork()) == -1)
                perror("Error when trying to fork");
            else if (newProcess == 0) {
                new_proc->pid = getpid();
                execvp(args[1], args+1);
                perror("Cannot execute command");
                printf("tsh>");
                exit(1);                
            }
        } else if(strcmp(firstArgument, "list") == 0) {
            print_procs((struct proc_info *) &anchor);
        } else if(strcmp(firstArgument, "info") == 0) {
            printf("ToDo");
        } else if(strcmp(firstArgument, "wait") == 0) {
            printf("ToDo");
        } else if(strcmp(firstArgument, "kill") == 0) {
            printf("ToDo");
        } else if(strcmp(firstArgument, "quit") == 0) {
            return 0;
        } else {
            int status = system(input);
            if(status != 0) {
                printf("--->[invalid command]<---\n");
            }
            printf("--->[status=%d]<---\n", status);     
        }

        //Free allocated memory for the arguments
        freemakeargv(args);
    }
}

void print_procs(struct proc_info *proc_list) {
    struct proc_info *elem = proc_list;
    elem = (struct proc_info *) elem->head.next;
    printf(" No | PID    | Running  | Status | Command\n");
    while(proc_list != elem) {
        printf("%3d | %6d | %8s | %6d | %s\n", elem->number, elem->pid, elem->running, elem->status, elem->command);
        elem = (struct proc_info *) elem->head.next;
    }
}

int makeargv(const char *s, const char *delimiters, char ***argvp) {
   int error;
   int i;
   int numtokens;
   const char *snew;
   char *t;

   if ((s == NULL) || (delimiters == NULL) || (argvp == NULL)) {
      errno = EINVAL;
      return -1;
   }
   *argvp = NULL;                           
   snew = s + strspn(s, delimiters);         /* snew is real start of string */
   if ((t = malloc(strlen(snew) + 1)) == NULL) 
      return -1; 
   strcpy(t, snew);               
   numtokens = 0;
   if (strtok(t, delimiters) != NULL)     /* count the number of tokens in s */
      for (numtokens = 1; strtok(NULL, delimiters) != NULL; numtokens++) ; 

                             /* create argument array for ptrs to the tokens */
   if ((*argvp = malloc((numtokens + 1)*sizeof(char *))) == NULL) {
      error = errno;
      free(t);
      errno = error;
      return -1; 
   } 
                        /* insert pointers to tokens into the argument array */
   if (numtokens == 0) 
      free(t);
   else {
      strcpy(t, snew);
      **argvp = strtok(t, delimiters);
      for (i = 1; i < numtokens; i++)
          *((*argvp) + i) = strtok(NULL, delimiters);
    } 
    *((*argvp) + numtokens) = NULL;             /* put in final NULL pointer */
    return numtokens;
}     

void freemakeargv(char **argv) {
   if (argv == NULL)
      return;
   if (*argv != NULL)
      free(*argv);
   free(argv);
}

dll.c:

#include <stdlib.h>
#include <stdio.h>
#include "dll.h"

/* initialize "shortcut links" for empty list */
void
list_init(struct list_head *head)
{
    head->prev = head;
    head->next = head;
}

/* insert new entry after the specified head */
void 
list_add(struct list_head *new, struct list_head *head)
{
    new->prev = head;
    new->next = head->next;
    head->next = new;
    new->next->prev = new;
}

/* insert new entry before the specified head */
void 
list_add_tail(struct list_head *new, struct list_head *head)
{
    //Insterting before head is equal to inserting after the previous of the head
    list_add(new, head->prev);
}

/* deletes entry from list and reinitialize it, returns pointer to entry */
struct list_head* 
list_del(struct list_head *entry)
{
    struct list_head *previous = entry->prev;
    struct list_head *following = entry->next;
    previous->next = following;
    following->prev = previous;
    entry->prev = NULL;
    entry->next = NULL;
    return entry;
}

/* delete entry from one list and insert after the specified head */
void 
list_move(struct list_head *entry, struct list_head *head)
{
    list_add(list_del(entry), head);
}

/* delete entry from one list and insert before the specified head */
void 
list_move_tail(struct list_head *entry, struct list_head *head)
{
    list_add_tail(list_del(entry), head);
}

/* tests whether a list is empty */
int 
list_empty(struct list_head *head)
{
    return (head == head->prev && head == head->next);
}

dll.h:

#ifndef DLL_H
#define DLL_H

struct list_head {
    struct list_head *next, *prev;
};

/* initialize "shortcut links" for empty list */
extern void
list_init(struct list_head *head);

/* insert new entry after the specified head */
extern void 
list_add(struct list_head *new, struct list_head *head);

/* insert new entry before the specified head */
extern void 
list_add_tail(struct list_head *new, struct list_head *head);

/* deletes entry from list and reinitialize it, returns pointer to entry */
extern struct list_head* 
list_del(struct list_head *entry);

/* delete entry from one list and insert after the specified head */
extern void 
list_move(struct list_head *entry, struct list_head *head);

/* delete entry from one list and insert before the specified head */
extern void 
list_move_tail(struct list_head *entry, struct list_head *head);

/* tests whether a list is empty */
extern int 
list_empty(struct list_head *head);

#endif

Answer 1

The mistake is, that you set the process id in the child

        if ((newProcess = fork()) == -1)
            perror("Error when trying to fork");
        else if (newProcess == 0) {
            // This is executed in the CHILD PROCESS!!
            new_proc->pid = getpid();
            execvp(args[1], args+1);
            perror("Cannot execute command");
            printf("tsh>");
            exit(1);                
        }

The child process doesn't share memory with the parent, so the information is not automagically transported to the parent. You have to add this after your if-block:

        new_proc->pid = newProcess;

This way the child pid is saved in the main process.

For the command-issue: The pointer is uninitialized:

char *command;
strncpy(command, input+4, len-4);

You have to allocate memory before copying to it and make sure, that the result is terminated with a null byte:

char *command = malloc(len-4+1);
strncpy(command, input+4, len-4);
command[len-4] = '\0';