Directory entries and linked lists

Question

This is code to put directory entry strings (from the root directory, in this case) into a single-linked list. I do not understand why a seg fault occurs at the line commented as such. I must be doing the string copying wrong?

I can only think that space for the string to go has not been reserved, but I thought I'd already reserved it by tmp1 = (struct node *)malloc(sizeof(struct node));?

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <dirent.h>
#include <string.h>

struct node {
    struct dirent *ent;
    struct node *right;
};

void appendnode(struct node **, struct dirent **);

int main() {
    DIR *d;
    struct dirent *d_ent;
    struct node *list;
    list = NULL;
    d = opendir("C:/");
    d_ent = readdir(d);
    while (d_ent) {
        appendnode(&list, &d_ent);
        d_ent = readdir(d);
    }
    getchar();
    return 0;
}

void appendnode(struct node **q, struct dirent **r) {
    struct node *tmp1, *tmp2;
    tmp1 = (struct node *)malloc(sizeof(struct node));
    strcpy(tmp1 -> ent -> d_name, (*r) -> d_name);  // <-- Causes seg fault. Why?
    tmp1 -> right = NULL;
    if (!*q) {
        *q = tmp1;
    } else {
        tmp2 = *q;
        while (tmp2 -> right) {
            tmp2 = tmp2 -> right;
        }
        tmp2 -> right = tmp1;
    }
}

`tmp1` contains a pointer to unitialised memory returned by `malloc()`. Accessing its value, which is what happens when evaluating `tmp1->ent` gives undefined behaviour. Evaluating `tmp1->ent` happens in order to evaluate `tmp1->ent->d_name` which is then passed to `strcpy()`. The net effect is that `strcpy()` is passed a bad pointer, and writes to it. — Peter, Jul 07 '18 at 04:21

H.S. · Accepted Answer · 2018-07-07T04:20:53.167

You are getting segmentation fault because you are trying to access an un-initialized pointer tmp1->ent:

strcpy(tmp1 -> ent -> d_name, (*r) -> d_name);
               ^^^

After allocating memory to tmp1, you should allocate memory to tmp1->ent also

tmp1->ent = malloc(sizeof(struct dirent));
if (tmp1->ent == NULL) {
    fprintf (stderr, "Failed to allocate memory");
    exit(EXIT_FAILURE);
}

Also, you don't need to cast the malloc return. Follow the good programming practice, always check for NULL to the pointer trying to allocate memory. You should do

if (tmp1 == NULL) {
    fprintf (stderr, "Failed to allocate memory");
    exit(EXIT_FAILURE);
}

Handy link, See: [Do I cast the result of malloc?](http://stackoverflow.com/q/605845/995714) — David C. Rankin, Jul 07 '18 at 05:35

score 0 · Answer 2 · answered Jul 07 '18 at 04:06

struct node {
    struct dirent *ent;
    struct node *right;
};

tmp1 = (struct node *)malloc(sizeof(struct node));

Print the sizeof(struct node) before calling malloc and see how much it shows. What you are doing here is allocating memory for a pointer to struct dirent and a pointer to struct node. Depending on the system you are running on, each pointer will have a size of 4 bytes or 8 bytes. You need to allocate memory for each directory entry in each node and use the pointer to point to that entry.

Taking a closer look at your code, I'm not really sure what is it that you are trying to achieve. The code you have pasted will be stuck in infinite loop after fixing memory allocation issues. I suggest you take a pen and paper and step through the program to understand what's happening.

Did you try debugging it? Does your program ever complete? Also, see [this](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) — , Jul 07 '18 at 17:50

Directory entries and linked lists

2 Answers2