Anonymous
Anonymous asked in Computers & InternetProgramming & Design · 1 month ago

C - handling memory leak?

I have a program below that manages a library full of Books and when I test for memory leaks using valgrind and then enter 'CTRL+Z' to quit, it says there are hundreds of bytes lost. I deallocated my code using 'free' at the end of main.c and I was wondering if there's something wrong with the way I'm doing it? I also use 'malloc' in addBook() and I'm not sure if I should free there as well. 

main.c

 int main(){

  LibraryType* library = malloc(sizeof(LibraryType));

  BookCollectionType* book = malloc(sizeof(BookCollectionType));

  addBook(book, "The Hunger Games", 2012, 405.5);

  addBook(book, "Twilight", 2007, 302);

  addBook(book, "The Martian", 2015, 203.2);

  }

  for(int i = 0; i < MAX_BOOKS; ++i){

   free(p->books[i]);

   free(library->bookCollection[i]);

  }

  free(book);

  free(library);

 }

library.c

 int addBook(BookCollectionType* bookCollection, char* t, int c, float p){

  bookCollection->books[bookCollection->numBooks] = malloc(sizeof(BookType));

  bookCollection->books[bookCollection->numBooks]->title = t;

  bookCollection->books[bookCollection->numBooks]->numCopies = c;

  bookCollection->books[bookCollection->numBooks]->pages = pages;

  return 1;

 }

defs.h

 typedef struct{

  char* title;

  int year;

  int numCopies;

  float pages;

 }BookType;

 typedef struct {

  BookType* books[MAX_PAGES];

  int numBooks;

 }BookCollectionType;

 typedef struct{

  char* libraryName;

  BookCollectionType* bookCollection[MAX_BOOKS];

 }LibraryType;

2 Answers

Relevance
  • 1 month ago

    The only memory leak I see is the failure to increment bookCollection->numBooks at the end of addBook().

    You have multiple other problems, but it's hard to tell because you seem to have left out a lot of code from main.c (the #includes, prototypes, if any, and most of the body of main()) but left in a stray } for confusion.

    What's left has what looks like a glaring error in the cleanup logic.  You appear to be freeing MAX_BOOKS books from a BookType object at the end, even though only the first (numBooks) entries in books[] actually have pointers in them.  Freeing pointer that hasn't been allocated (or has already been freed) is undefined behavior; usually crashing the program.

    Your code has other problems, maybe worse ones.  The "defs.h" file (should have a better name!) should include the prototypes for functions related to those struct types, and also contain the global constants like MAX_BOOKS and MAX_PAGES.  Having #defines for these in  each .c file in your project creates the possibility that some update might give different values, leading to hard-to-find bugs and mystery crashes.

    As a final note, think about adding functions to allocate (and initialize!) your different data structure types.  There's no object-oriented programming support in C, so you have to write functions to act as methods and constructors.

    Final thought: Use calloc(), not malloc() to allocate memory.  That will set the memory area to binary zeroes, making all pointers NULL, all numbers 0 or 0.0, and all char[] strings empty.  If nothing else, that does part of the work of a constructor.

  • User
    Lv 7
    1 month ago

    Your "for" loop, including all of the "free" commands, are outside of the "main" function. Seems like those lines will never be "run".

Still have questions? Get your answers by asking now.