Jump to content
  • Advertisement
Sign in to follow this  
Alpha_ProgDes

[C] *string[] is not working with me

This topic is 3881 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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.
#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;
    }
}

Share this post


Link to post
Share on other sites
Advertisement
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).

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
Quote:
Original post by Alpha_ProgDes
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.
Are you saying you now have:
static char *parsed_command[256] = {1};
?

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
Quote:
Original post by jyk
Quote:
Original post by Alpha_ProgDes
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.
Are you saying you now have:
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;
}
}

Share this post


Link to post
Share on other sites

// 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 them

void 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" command
int 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;
}




Share this post


Link to post
Share on other sites
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)

#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);
}
}

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
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.


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!

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!