Segmentation fault (core dumped)

Question

I'm writing a program in c that basically copies files, but I'm getting this error: Segmentation fault (core dumped). From what I'm reading I think it's because I'm trying to access memory that hasn't been allocated yet. I'm a newbie when it comes to c and I suck at pointers, so I was wondering if you guys could tell me which pointer is causing this and how to fix it if possible. Btw, this program is supposed to be a daemon, but I haven't put anything inside the infinite while loop at the bottom. Here is my code:

#include <sys/types.h>
#include <sys/stat.h>
#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include <errno.h>
#include <unistd.h>
#include <syslog.h>
#include <string.h>
#include <dirent.h>

int main(int c, char *argv[]) {
char *source, *destination;
char *list1[30], *list2[30], *listDif[30];
unsigned char buffer[4096];
int i=0, x=0, sizeSource=0, sizeDest=0, sizeDif=0;
int outft, inft,fileread;
int sleeper;
struct dirent *ent, *ent1;

//Check number of arguments
if(c<3) 
{
    printf("Daemon wrongly called\n");
    printf("How to use: <daemon name> <orginDirectory> <destinationDirectory> \n");
    printf("or : <daemon name> <orginDirectory> <destinationDirectory> <sleeperTime(seconds)>");
    return 0;
}

//Checks if sleeper time is given or will be the default 5minutes
/*if(c=4)
{
    char *p;
    errno = 0;
    long conv = strtol(argv[3], &p, 10);
    if(errno != 0 || *p != '\0') 
    {
        printf("Number given for sleeper incorrect, it has to be an integer value.\n");
        return(0);
    } else 
    {
    sleeper = conv;
    }
} else
{
    sleeper = 300;
}*/

//Get path of directories from arguments
source = argv[1];
destination = argv[2];

//Check if directories exist
DIR* dirSource = opendir(source);
if (!dirSource)
{
    printf("Source directory incorrect\n");
    return 0;
}
DIR* dirDest = opendir(destination);
if (!dirDest)
{
    printf("Destination directory incorrect\n");
    return 0;
}

/* save all the files and directories within directory */
while ((ent = readdir (dirSource)) != NULL) {
    list1[sizeSource] = strdup(ent->d_name);    
    sizeSource++;
    if(sizeSource>=30){break;}
}
closedir(dirSource);
while((ent1 = readdir (dirDest)) != NULL) {
    list2[sizeDest] = strdup(ent1->d_name);
    sizeDest++;
    if(sizeDest>=30){break;}
}
closedir(dirDest);
/* Verify the diferences between the directories and save them */
int z;
int dif = 0; //0 - False | 1 - True
printf("Diferenças:\n");
for(i=0;i<sizeSource;i++){
    dif = 0;
    for(z=0;z<sizeDest;z++){
        if(strcmp(list1[i],list2[z])==0){ //If there is no match, it saves the name of the file to listDif[]
            dif = 1; 
            break;
        }   
    }
    if(dif==0) {
        printf("%s\n",list1[i]);
        listDif[sizeDif] = list1[i];
        sizeDif++;
    }
}

/* This code will copy the files */
z=0;
while(z!=sizeDif){
    // output file opened or created
    char *pathSource, *pathDest;        

    strcpy(pathSource, source);
    strcat(pathSource, "/");
    strcat(pathSource, listDif[z]);

    strcpy(pathDest, destination);
    strcat(pathDest, "/");
    strcat(pathDest, listDif[z]);

    // output file opened or created
    if((outft = open(pathDest, O_CREAT | O_APPEND | O_RDWR))==-1){
        perror("open");
    }
    // lets open the input file
    inft = open(pathSource, O_RDONLY);
    if(inft >0){ // there are things to read from the input
        fileread = read(inft, buffer, sizeof(buffer));
        printf("%s\n", buffer);
        write(outft, buffer, fileread);
        close(inft);
    }
    close(outft);
}



/* Our process ID and Session ID */
pid_t pid, sid;

/* Fork off the parent process */
pid = fork();
if (pid < 0) {
        exit(EXIT_FAILURE);
}
/* If we got a good PID, then
   we can exit the parent process. */
if (pid > 0) {
        exit(EXIT_SUCCESS);
}

/* Change the file mode mask */
umask(0);

/* Open any logs here */        

/* Create a new SID for the child process */
sid = setsid();
if (sid < 0) {
        /* Log the failure */
        exit(EXIT_FAILURE);
}



/* Change the current working directory */
if ((chdir("/")) < 0) {
        /* Log the failure */
        exit(EXIT_FAILURE);
}

/* Close out the standard file descriptors */
close(STDIN_FILENO);
close(STDOUT_FILENO);
close(STDERR_FILENO);

/* Daemon-specific initialization goes here */

/* The Big Loop */
while (1) {

   //sleep(5); /* wait 5 seconds */
}
exit(EXIT_SUCCESS); 

} The result of ls is:

ubuntu@ubuntu:~/Desktop$ ls
Concatenar_Strings.c   core  D2      daemon.c   examples.desktop
Concatenar_Strings.c~  D1    daemon  daemon.c~  ubiquity.desktop

D1 and D2 are folders, and in D1 are three text documents that I want to copy into D2. One other question, is this a delayed error or an immediate one? Because I doubt this message would appear on a code line that with two integers. Thanks in advance guys.

Answer 1

This loop is wrong:

while ((ent = readdir (dirSource)) != NULL) {
    list1[sizeSource] = ent->d_name;    

Probably, ent points to the same memory block every time, and the readdir function updates it. So when you save that pointer, you end up with your list containing invalid pointers (probably end up all pointing to the same string). Further, the string may be deallocated once you got to the end of the directory.

If you want to use the result of readdir after closing the directory or after calling readdir again you will need to take a copy of the data. In this case you can use strdup and it is usually good style to free the string at the end of the operation.

This may or may not have been the cause of your segfault. Another thing to check is that you should break out of your loops if sizeSource or sizeDest hits 30.

In the strcmp loop, you should really set dif = 0 at the start of the i loop, instead of in an else block.

---

Update: (more code shown by OP)

char *pathSource, *pathDest;
strcpy(pathSource, source);

You are copying to a wild pointer, which is a likely cause of segfaults. strcpy does not allocate any memory, it expects that you have already allocated enough.

One possible fix would be:

char pathSource[strlen(source) + 1 + strlen(listDif[z]) + 1];
sprintf(pathSource, "%s/%s", source, listDif[z]);

Alternatively (without using VLA):

char pathSource[MAX_PATH];   // where MAX_PATH is some large number
snprintf(pathSource, MAX_PATH, "%s/%s", source, listDif[z]);

Do the same thing for pathDest.

NB. Consider moving the closedir lines up to after the readdir loops; generally speaking you should open and close a resource as close as possible to the times you start and finish using them respectively; this makes your code easier to maintain.