#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/wait.h>
void read_command();
void exec_cmdline();
static char *parsed_command[256] = {0};
int main (int argc, char *argv[])
{
char prompt[256];
int exiting;
strcpy(prompt, strcat(argv[0], ">"));
if (argc != 2)
{
printf("There can only be one argument. Please rerun program.\n");
return 0;
}
while (exiting = strcmp(parsed_command[0], "exit") != 0) //compilers don't like this
{
printf("%s", prompt);
read_command();
exec_cmdline();
}
return 0;
}
void read_command()
{
int end = 2;
char command_buffer[256];
char *delim = " ";
fgets(command_buffer, 256, stdin);
parsed_command[0] = strtok(command_buffer, delim);
parsed_command[1] = parsed_command[0];
while (parsed_command[end] != NULL)
{
parsed_command[end] = strtok(NULL, delim);
++end;
}
}
void exec_cmdline()
{
pid_t pid;
switch (pid = fork())
{
case -1: printf("Fork() has errored.\n"); break;
case 0: execvp(parsed_command[0], parsed_command);
exit(0); break;
default: waitpid(pid, NULL, 0); break;
}
}
[C] *string[] is not working with me
the problem is when the program gets to parsed_command[0], i get one compiler window open and close immediately and another compiler give me a segmenation fault error. so i don't know what i'm doing wrong.
parsed_command[256] is an array of pointers. You have initialized the value of each of these pointers to 0 by writing:
static char *parsed_command[256] = {0}
Next without having any of those pointers point at anything you somewhat perplexedly perform strcmp(parsed_command[0], "exit") . The function dereferences the pointer at position 0, which is NULL, in an attempt to compare it to the string "exit". Dereferencing a NULL pointer then produces your segmentation fault.
Just by examining your code very briefly it seems like you can fix it by:
do
{
printf("%s", prompt);
read_command();
exec_cmdline();
}while (exiting = strcmp(parsed_command[0], "exit") != 0);
...
Almost all of C's original raw memory operations aren't particularly 'safe', as I'm sure you've just noticed. Even the new 'safe' versions, are only slightly safer (as safe as the C programming language can possibly allow).
static char *parsed_command[256] = {0}
Next without having any of those pointers point at anything you somewhat perplexedly perform strcmp(parsed_command[0], "exit") . The function dereferences the pointer at position 0, which is NULL, in an attempt to compare it to the string "exit". Dereferencing a NULL pointer then produces your segmentation fault.
Just by examining your code very briefly it seems like you can fix it by:
do
{
printf("%s", prompt);
read_command();
exec_cmdline();
}while (exiting = strcmp(parsed_command[0], "exit") != 0);
...
Almost all of C's original raw memory operations aren't particularly 'safe', as I'm sure you've just noticed. Even the new 'safe' versions, are only slightly safer (as safe as the C programming language can possibly allow).
ok. i figured it out. initialize a static char string to 0 makes everything NULL. and strcmp doesn't like NULL. i changed the initialization to "1" and it works better.
but now my comparison isn't working. i type "exit" at the prompt and i still get the prompt. it doesn't exit.
but now my comparison isn't working. i type "exit" at the prompt and i still get the prompt. it doesn't exit.
Quote:Original post by Alpha_ProgDesAre you saying you now have:
ok. i figured it out. initialize a static char string to 0 makes everything NULL. and strcmp doesn't like NULL. i changed the initialization to "1" and it works better.
static char *parsed_command[256] = {1};Quote:strcmp(parsed_command[0], "exit")
What does parsed_command[0] equal? have you tried printing it out? strtok() might be returning a pointer to inside of the string you gave it not a new string.
Quote:Original post by jykQuote:Original post by Alpha_ProgDesAre you saying you now have:
ok. i figured it out. initialize a static char string to 0 makes everything NULL. and strcmp doesn't like NULL. i changed the initialization to "1" and it works better.?static char *parsed_command[256] = {1};
If you've done what jyk thinks you've done then you've simply induced undefined behavior. The value at parsed_command must be an address pointing the beginning of some contiguous memory which is terminated by a zero byte. So until you assign parsed_command to some value obtained by malloc or new, its value should otherwise be zero to indicate that it is uninitialized. You can't/shoudn't manually assign a pointer to be some arbitrary value other than 0.
Additionally there is issue with the function read_command() which will surely cause issues soon. parsed_command is being made to point to stack memory. So when you come to access parsed_command inside the main while loop in main you'll encounter more undefined behavior.
void read_command(){ int end = 2; char command_buffer[256];// parsed_command ends up pointing at this array which resides on the stack!!! char *delim = " "; fgets(command_buffer, 256, stdin); parsed_command[0] = strtok(command_buffer, delim); parsed_command[1] = parsed_command[0]; while (parsed_command[end] != NULL) { parsed_command[end] = strtok(NULL, delim); ++end; }}// Sorry for the C++-styled comments. ;)#include <unistd.h>#include <stdio.h>#include <stdlib.h>#include <string.h>#include <sys/wait.h>// Pass things around so you don't need globals. Make a structure for arguments:typedef struct parsed_command_s { char words[256][256]; // since your existing code suggests that you // aren't going to be able to handle the dynamic allocation properly ;)} parsed_command;// Functions up here, so you don't need to declare themvoid read_command(char* prompt, parsed_command* command){ // show prompt // read into local buffer // parse into the pointed-at parsed_command - strcpy into the appropriate // 'words'}// do the check here for whether we have a "special" commandint exec_cmdline(parsed_command* command){ pid_t pid; if (!strcmp(command->words[0], "exit")) { return 0; } switch (pid = fork()) { case -1: printf("Fork() has errored.\n"); break; case 0: execvp(command->words[0], command->words); exit(0); // 'break' here is redundant default: waitpid(pid, NULL, 0); return 1; }}int main (int argc, char *argv[]){ char prompt[256] = {0}; // The previous way was not kosher: you can't strcat onto argv elements // because you don't necessarily own the memory that the concatenated stuff // will take up. What we will do instead is "concatenate" the argument // "to the beginning" of the prompt buffer, and use the returned value (end // of the concatenation) as the location to copy the angle bracket (to avoid // an extra strlen operation). A completely ridiculous optimization in this // context, of course, but that IS why you're using C, yes? :) strcpy(strcat(prompt, argv[0]), ">"); if (argc != 2) { printf("There can only be one argument. Please rerun program.\n"); // This is an error condition, so you should return something non-zero // from main(). return 1; } // See how neatly this reads now: parsed_command pc; do { read_command(prompt, &pc); } while (exec_cmdline(&pc)); return 0;}
Thanks everybody for your help. I never worked much with ** or *[] types of C-strings. But thanks for unconfusing me. And to Zahlman [razz], I did get the pointers to work!
Working code. Feel free to critique (heavily if necessary)
Working code. Feel free to critique (heavily if necessary)
#include <unistd.h>#include <stdio.h>#include <stdlib.h>#include <string.h>#include <sys/wait.h>void read_command();int exec_cmdline ();void initialize ();void release ();static char *parsed_command[16] = {0};int main (int argc, char *argv[]){ char prompt[256]; int exiting; char end; strcpy(prompt, argv[0]); strcat(prompt, ">"); /* if (argc != 2) { printf("There can only be one argument. Please rerun program.\n"); return 0; } */ initialize(); do { printf("%s", prompt); read_command(); } while (exec_cmdline() != 0); release(); printf("\nFinished.\n"); scanf("%c", &end); return 0;}void read_command(){ int end = 1; char command_buffer[32]; char *delim = " " , *newline = NULL, *token; fgets(command_buffer, 32, stdin); newline = strchr(command_buffer, '\n'); if (newline) *newline = '\0'; strcpy(parsed_command[0], strtok(command_buffer, delim)); while (token = strtok(NULL, delim)) { strcpy(parsed_command[end], token); ++end; } parsed_command[end] = NULL; }int exec_cmdline(){ pid_t pid; if (strcmp(parsed_command[0], "exit") == 0) return 0; switch (pid = fork()) { case -1: printf("Fork() has errored.\n"); return 0; case 0: execvp(parsed_command[0], parsed_command); _exit(1); default: wait(NULL); return 1; }}void initialize (){ int i; for (i = 0; i < 16; ++i) { parsed_command = malloc(sizeof(char)*32); }}void release (){ int i; for (i = 0; i < 16; ++i) { free(parsed_command); }}Quote:Original post by Alpha_ProgDes
Thanks everybody for your help. I never worked much with ** or *[] types of C-strings. But thanks for unconfusing me. And to Zahlman [razz], I did get the pointers to work!
You got *allocation* working, but in this particular case, it's a useless piece of allocation work, because you always allocate a specific sized chunk of memory, and you allocate it in every slot. This adds complexity without actually gaining the flexibility that dynamic allocation can deliver. It's actually very difficult to do these things in a way that is useful and efficient, which is why (a) more modern languages provide library support for it, and (b) C programmers have a reputation for not bothering, and consequently suffering.
Quote:Original post by ZahlmanQuote:Original post by Alpha_ProgDes
Thanks everybody for your help. I never worked much with ** or *[] types of C-strings. But thanks for unconfusing me. And to Zahlman [razz], I did get the pointers to work!
You got *allocation* working, but in this particular case, it's a useless piece of allocation work, because you always allocate a specific sized chunk of memory, and you allocate it in every slot. This adds complexity without actually gaining the flexibility that dynamic allocation can deliver. It's actually very difficult to do these things in a way that is useful and efficient, which is why (a) more modern languages provide library support for it, and (b) C programmers have a reputation for not bothering, and consequently suffering.
LOL, I would call you a hater, but you're right I could have done [256][256] (or something similar). But thanks for the help. Now on to the link!
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
