Writing my own shell

[lexion]

I am having trouble with a shell that I am writing for fun. When I type the following:
I get the error at the end:

/home/me/code/ish - ish-0.1> ls
a.out main.c
/home/me/code/ish - ish-0.1> cd ..
/home/me/code - ish-0.1> ls
ls: cannot access : no such file or directory

Why doesn't it work? Here is my code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

#define N_ARGS 64
#define L_IS 256
#define L_OS 1024
#define L_ARG 32

#define PROMPT "ish-0.1> "

char *pa[N_ARGS]; // Parsed Arguments
char *is; // Input String
char *ts; // Temp String
char *cwd; // current working dir

int main(int argc, char *argv[], char *envp[])
{
	cwd = (char *)malloc(L_IS);
	is = (char *)malloc(L_IS);
	int err = 1;
	int i = 1;

	while(1)
	{
		// Reset all variables/strings/arrays
		memset(is, 0, L_IS);
		for (i = 0; i < N_ARGS && pa[i] != NULL; i++)
			if (pa[i] != NULL)
				memset(pa[i], 0, strlen(pa[i]));
		i = 1;

		// Print the prompt and get user input
		getcwd(cwd, L_IS);
		printf("%s - %s", cwd, PROMPT);
		fgets(is, L_IS, stdin);
		*(strchr(is, '\n')) = 0;

		// Parse the user input
		for (pa[0] = strtok(is, " "); ts = strtok(NULL, " "); i++)
			pa[i] =  ts;

		// Take care of 'special' shell-defined commands
		if (strstr(pa[0], "exit") != NULL)
			return 0;
		else if (strstr(pa[0], "cd") != NULL)
		{
			chdir(pa[1]);
			continue;
		}

		// Execute the 'real' commands
		if (fork() == 0)
		{
			err = execvp(pa[0], pa);
			if (err < 0)
				printf("%s is not a known command\n", pa[0]);
			exit(1);
		}
		else
			wait(1); // Give the process time to start
	}

	return 0;
}

[gerard4143]

I just tried you code and it worked on my machine....That is I ran it and tried ls then cd .. ls and it produced correct results...Gerard4143

[lexion]

Can I see the output of your 'which cd' please?

[Rubberman]

A couple of problems.

1. When you reset your variables, you need to set the entries of array pa[i] to NULL, not to memset each non-null entry. You do that when you memset(is, 0, L_IS) before. What you are doing is leaving dirty pointers in the array.

 for (i = 0; i < N_ARGS; i++) { pa[i] = 0; }

2. When you parse the user input, you should either change your loop or explicitly NULL terminate the array.

for (i = 0, ts = strtok(is, " "); ts != 0; i++)
{
    pa[i] = ts;
    ts = strtok(NULL, " ');
}
pa[i] = 0;

[lexion]

Thank you rubberman. That worked.

[Rubberman]

Thank you rubberman. That worked.

Glad to help. There are a lot of placed in C code where null termination of arrays of pointers is critical, otherwise you end up munging your memory, or try to access invalid memory, resulting in a segfault.