patterncMinor
Recovery Shell (rsh)
Viewed 0 times
recoveryrshshell
Problem
I wrote a shell to test my understanding of processes etc. It is not supposed to comply with POSIX or anything, just to allow people to run simple commands with simple arguments.
```
#include / printf, fprintf /
#include / exit /
#include / strtok /
#include / fork, execvp /
#include / waitpid /
#include / waitpid /
/*
* This program is a simple, first-level shell for testing and development purposes.
* It is called rsh: the Recovery SHell because it can be used for recovery as it has
* no dependencies apart from the C standard library and is very small and simple.
* This shell does not follow POSIX or anything else: it is purely for executing
* commands with simple arguments (ie no quote escape, backslash escape, etc)
*/
/*
* Function Return Values:
* All functions in this program return 0 on success and -1 on failure, unless they
* return useful information, in which the return values will be documented in a
* comment at the top of the function. With the exception of builtins. Builtin
* commands return 0 for success and 1 for not successful, like the real commands.
*/
/ A list of builtin functions here /
char *builtins[] = {
"exit",
"cd",
"help"
};
/*
* Return the number of builtin commands so we can avoid hard-coding random magic
* numbers and other annoyances everywhere.
*/
int rsh_num_builtins(void)
{
return sizeof(builtins) / sizeof(char *);
}
/*
* This builtin function exits the shell.
*/
int rsh_exit(char **args)
{
exit(EXIT_SUCCESS);
}
/*
* This builtin function changes the directory of the shell process, like the cd
* in normal shells.
*/
int rsh_cd(char **args)
{
if (args[1] == NULL) {
fprintf(stderr, "rsh: expected argument to cd\n");
return 0;
} else if (chdir(args[1]) == -1) {
fprintf(stderr, "rsh: can't cd to %s\n", args[1]);
return 1;
}
return 0;
}
/*
* Print out a help message about rsh
*/
int rsh_help(char **args)
{
puts("rs
```
#include / printf, fprintf /
#include / exit /
#include / strtok /
#include / fork, execvp /
#include / waitpid /
#include / waitpid /
/*
* This program is a simple, first-level shell for testing and development purposes.
* It is called rsh: the Recovery SHell because it can be used for recovery as it has
* no dependencies apart from the C standard library and is very small and simple.
* This shell does not follow POSIX or anything else: it is purely for executing
* commands with simple arguments (ie no quote escape, backslash escape, etc)
*/
/*
* Function Return Values:
* All functions in this program return 0 on success and -1 on failure, unless they
* return useful information, in which the return values will be documented in a
* comment at the top of the function. With the exception of builtins. Builtin
* commands return 0 for success and 1 for not successful, like the real commands.
*/
/ A list of builtin functions here /
char *builtins[] = {
"exit",
"cd",
"help"
};
/*
* Return the number of builtin commands so we can avoid hard-coding random magic
* numbers and other annoyances everywhere.
*/
int rsh_num_builtins(void)
{
return sizeof(builtins) / sizeof(char *);
}
/*
* This builtin function exits the shell.
*/
int rsh_exit(char **args)
{
exit(EXIT_SUCCESS);
}
/*
* This builtin function changes the directory of the shell process, like the cd
* in normal shells.
*/
int rsh_cd(char **args)
{
if (args[1] == NULL) {
fprintf(stderr, "rsh: expected argument to cd\n");
return 0;
} else if (chdir(args[1]) == -1) {
fprintf(stderr, "rsh: can't cd to %s\n", args[1]);
return 1;
}
return 0;
}
/*
* Print out a help message about rsh
*/
int rsh_help(char **args)
{
puts("rs
Solution
Realloc strategy
I'm not a fan of how you are reallocing your token array to be one element bigger on each new token. That turns into an \$O(N^2)\$ task. I would recommend either:
Another thing you could do is to reuse the token array and not free it every time.
DIM macro
You have a function
Error handling
All of your functions return error values or NULL, but
If you don't actually care about
Well written
Your code is written clearly with good comments. I had no trouble reading and understanding it. So good job on that.
I'm not a fan of how you are reallocing your token array to be one element bigger on each new token. That turns into an \$O(N^2)\$ task. I would recommend either:
- Do one quick pass over the input to count the number of tokens.
- Doubling the size of the token array each time.
Another thing you could do is to reuse the token array and not free it every time.
DIM macro
You have a function
rsh_num_builtins(), which I would normally handle with a DIM macro:#define DIM(array) (sizeof(array)/sizeof(array[0]))
for (i = 0; i < DIM(builtins); i++) {Error handling
All of your functions return error values or NULL, but
main never checks if line is NULL, or uses the return value of rsh_execute().If you don't actually care about
rsh_execute()'s return value, it might be more clear to just change that function to return void. However it seems like in your case you might later extend the shell to use the return value, so it's probably ok the way you have it.Well written
Your code is written clearly with good comments. I had no trouble reading and understanding it. So good job on that.
Code Snippets
#define DIM(array) (sizeof(array)/sizeof(array[0]))
for (i = 0; i < DIM(builtins); i++) {Context
StackExchange Code Review Q#90787, answer score: 7
Revisions (0)
No revisions yet.