Segmentation Fault, and I know where it is

[piero.costa]

Hi guys,

I'm having a weird error that I can't understand why:
My program is the following:

int paralelo(int num)
{
int cont,pid;
    printf("Creating %d sons with pid\n", num);
    for(cont=0;cont<num;cont++)
        {
        pid=fork();
        if(pid==0)
            {
            printf("I'm the son number %d e  %d\n", cont, getpid());
            exit(0);
            }
        }
return(0);
}       



int main(int argc, char* argv[])
{
int k=0, j, i, x, m, n,status, cont;
char* comandi[256];

k=atoi(argv[1]);

for(cont=0;cont<k;cont++)
{
strncpy(comandi[cont], argv[cont+1], sizeof(comandi[n]));
}
x=paralelo(k);
}

The problem comes when I execute the program:
./program 3 file1 file2 fil3

When I put as argv[1] any number, I get a segmentation fault. If I don't use any numbers I don't get any errors. But why ? I'm supposed to execute the program giving the program name, a constant ( argv[1] ) and then "n" file names.
Why am I getting this error ? It could be because Im using portable ubuntu on windows or I'm saying non senses ?

thanks

[Ancient Dragon]

you need to check the value of argc before using argv. argv[1] is valid only if argc > 1

line 28: you are attempting to copy argv[count+1] to a pointer that does not have any memory allocated to it. strncpy() does NOT allocate memory for the string, you have to do that yourself. There are two ways to accomplish that

1. call malloc() to allocate memory for the string, then call strcpy() to copy it. strncpy() is not useful here.
2. call strdup(), which combines both the above

[piero.costa]

I still don't get it. If strdup duplicate and allocate the memory... I don't get why I'm getting a segmentation fault.
(forget about the value check, I'll do it later :P )

int main(int argc, char** argv)
{
int j, i, x, m, n,status, cont;
int k;
char* comandi[n];

n=argc;
k=atoi(argv[1]);

for(cont=0;cont<k;cont++)
{
comandi[i]=strdup(argv[i+1]);
}
}

Ok, let's see if I get it:
-I created a vector of pointers ( comandi[n] )
-I get the first value that is passed to the command like and I transform it into a integer
-The last thing I do a for cicle that will duplicate and allocate the pointers of argv[] in my new vector of pointers.

The idea seems right, the commands too. But when I run, I still get a segfault.

???

[Ancient Dragon]

Try this one, here we ignore the value of argv[1] because it's not needed.

char* comandi[n] = {NULL}; // initialize all pointers to NULL
for(cont=2;cont<argc;cont++)
{
comandi[i-2]=strdup(argv[i]);
}

[WaltP]

In the loop what is the value of i?

[piero.costa]

WaltP, it was CONT actually, not i

@Ancient, this part works... But is it not allowed to do k=atoi(argv[1]) ??
what is going on in here ?

[piero.costa]

I don't understand why the k=atoi(argv[1]) is not working !!!!!!!
without it normally the program works fine if you initialize the pointers with NULL

[Ancient Dragon]

what do you mean "it's not working" ? That statement itself should not produce a seg fault or other error.

[WaltP]

WaltP, it was CONT actually, not i

Not in the code you posted. We can't tell if you post bad code.

I don't understand why the k=atoi(argv[1]) is not working !!!!!!!

How do you know it's not? What was the value of k when you tested it by printing it out?

[Banfa]

At line 5 you have char* comandi[n]; but at this point n is not initialised so even if you are using C99 (as opposed to C89/C90 which doesn't allow variable length arrays) this line of code can not go well.

Change the n for a 10 and everything works.

You do not return anything from main.

Since argc carries the number of arguments you arguably do not need to give the number of files on the command line as a separate argument (assuming that is what the 3 is)

In your loop you loop from 0 to k-1 (<k). for the command line '3 file1 file2 file3' that means copying 3, file1 and file2 but I get the impression you meant to copy file1, file2, file3.

[piero.costa]

Ok, thank you guys. Now it's working. The problems was because I had to initialize the vector char* comandi !
thank you all for the help!!

[piero.costa]

Just to not create another topic, I'll post in here.

I did all this because I have to create a program that works like that:

program_name k file1 file2 ... filenN

When the program is called with a value for K the program will create "K" sons with a fork() and they will paralellaly execute the first K file names in the argv[] array.

I've concluded the first part, that is passing the argv[1] value to the int K and the other file names to char* comandi[255]

Afterwards, I'd call a function called paralelo(k, comandi[255]) that will create "k" sons where each son will execute one file.

int paralelo(int num, char executable[255])
{
int i,pid,status, morte;
char files[255];

    printf("Serao criados %d filhos em paralelo\n", num);
    for(i=0;i<num;i++)
        {
        pid=fork();
        if(pid==0)
            {

            execl("????"); // how should I write this line to execute each line of executable[] ??
            exit(0);
            }
        if(pid!=0)
            {
            morte=wait(&status);
            printf("died son %d\n", morte);
            }
        }
return(0);
}       



int main(int argc, char** argv)
{
int j, x, m, n, cont;
int k;
char* comandi[255] = {NULL};


k=atoi(argv[1]);

printf("K vale %d\n", k);

for(cont=0;cont<k+1;cont++)
    { 
    comandi[cont]=strdup(argv[cont+1]);
    printf("File: %s\n ", comandi[cont]);
    }
x=paralelo(k, comandi[255]);
if(x==0)
    {
        printf("program ok\n");
        return(0);
    }
if (x!=0)
    {
        printf("error\n");
        return(1);  
    }
}

So, I need to create k sons where each son will execute one file name who was in argv[] and then passed to comandi[], where finally was passed to paralelo(int n, char executable[255]). Is my logic ok ? How could I do that ?

[Ancient Dragon]

Are you required to call excel()? If not, a simpler function is system(), such as system(executable);

[piero.costa]

No, I could use something else.
Ancient Dragon, is the function call ok ? I don't think I understand how the pointers are working in here.

1- char* comandi[255]
is an array of pointers that after the cicle for they will point to argv[]

2- when I call the function paralelo(k, comandi[255]), im giving to the function int paralelo(int num, char executable[255]) the same array of pointers - comandi[255] where the file names were stored, right ?

I get an error when inside paralelo() I try to printf("%s", executable[0]) to see if my idea was right.

warning: format ‘%s’ expects type ‘char *’, but argument 2 has type ‘int’

when I put & in front of executable[0] of course I don't get this error anymore but I still get a segmentation error even if I use malloc().

sorry for all these questions but I have never had C classes and all i learnt was online so... I'm still learning! ;)

[Ancient Dragon]

when I call the function paralelo(k, comandi[255]),

You don't call it like that: paralelo(k, commandi); That will pass all the command pointers to the function. If you want to pass only one of the pointers then its like this: paralelo(k, commandi[0]); which will pass commandi[0] pointer. You can substitute a loop counter for 0 in that statement.

You really don't even need that commandi list of pointers. Just pass argc and argv to function paralelo() and let it grab the names of file1 ... from argv itself. No need for copying the strings into commandi.

[piero.costa]

so it means if I have 5 files, I have to call the function 5 times?
I cannot pass an array to the function ?

[Ancient Dragon]

The first example I posted passes them all at the same time. You don't specify the size of the array, just the array name.

[piero.costa]

Edit:
ok, i'll try passing all of them

[piero.costa]

thanks again! You guys are saving me, you have no idea.

[piero.costa]

Ok, now I just have no idea how to create a serie of children using fork.
For exemple, if a number is given by the user, lets assume 4:

I'd have to do something like
child1 - creates -> child 2 - creates -> child3 - creates -> child4.

and each child should execute one different file whose name is given in argv[].

I have no idea how to do that, since the number will be given by the user!

[Ancient Dragon]

Are all childrden created by the main program, or does each child create yet another child? It would seem more reasonable for the main program to create all the children in a loop and pass one filename to each child.

I don't do *nix so I'm not certin if the following code will work or not. You may have to put a sleep() after each fork() so that the children don't get launched to quickly.

int main(int argc, char* argv[])
{
    int i;
    int id = 2;
    for(i = 2; i < argc; i++)
    {
        pid_t pid = vfork();
        if( pid == 0)
        {
            char* filename = argv[id];
            id++; // ready for next fork
            // process this filename
        }
    }

}