0

I am currently working on a virtual Filesystem. There are no actual physical folders or files, its just about practising data structures. To create a File or a Folder i have this struct:

struct FS_Object {
    int32_t id;
    struct FS_Object* parent;
    dataType dataType;
    FolderInfo  folderInfo;
    FileInfo    fileInfo;
};

FolderInfo and FileInfo are structs that contain an ID, a name and some other attributes.
Im storing every created struct in a global array to have access to the properties:

FS_Object* dataList;

So if I want to free a previous created FS_Object (lets say stored in dataList[1]):

FS_Objects are created like this:

FS_Object fsObject = malloc(sizeof(struct FS_Object));


1. Check if the element exists

if (dataList[1] != NULL) ...


2. Free FolderInfo struct from FS_Object struct like this:

FolderInfo *folderInfo = &dataList[1]->folderInfo;
free(folderInfo);
folderInfo = NULL;


3. Free FileInfo:

fileInfo *fileInfo = &dataList[1]->fileInfo;
free(fileInfo);
fileInfo = NULL;


4. Finally free the FS_Object struct itself:

free(dataList[1]);
dataList[1] = NULL;


Is this the right approach? I am currently getting the following exception when I'm trying to free the FolderInfo struct: 

*** Error in `./filesystem': free(): invalid pointer: 0x0000000001fa70e8 ***
dandan78
  • 13,328
  • 13
  • 64
  • 78
Marley
  • 185
  • 2
  • 2
  • 13
  • There is a lot that seems wrong in your code, but I think this question should better be asked in codereview.stackexchange.com. – Erich Kitzmueller Oct 29 '14 at 16:32
  • You are malloc'ing memory for an FS_Object, then trying to free memory in the middle of it ("folderInfo"). You can't do this. It seems very likely that you INTENDED for "folderInfo" and "fileInfo" to be pointers. – Taylor Brandstetter Oct 29 '14 at 16:36
  • No, your approach is not right. `folderInfo` and `fileInfo` are nested in `FS_Object` - they aren't pointers you obtained from `malloc` so you don't need to free them. You should only be `free`ing the `FS_Object` instances you `malloc`. – nobody Oct 29 '14 at 16:36
  • Good rule of thumb is, free everything in reverse order of the way you malloc memory – LeatherFace Oct 29 '14 at 16:42
  • Your global array should be an array of pointers, e.g. `FS_Object *dataList[100]`. Or you can declare `dataList` as `FS_Object **dataList`, and then `malloc` space for it. – user3386109 Oct 29 '14 at 16:45
  • I am using malloc to create space for my dataList like this: dataList = calloc(MAX_OBJECTS, sizeof(struct FS_Object)); – Marley Oct 29 '14 at 16:47

1 Answers1

1

If folderInfo is a bare struct (in opposition to a pointer to a struct), you shouldn't need to free it (it is allocated when you malloc FS_Object). Since you didn't malloc it, you don't need to free it.

So you can basically remove steps in number (2) and (3).

Check this question: Malloc of arrays and structs within a struct

Community
  • 1
  • 1
Márcio Paiva
  • 983
  • 9
  • 25