0

can you help explain to me why my following code has a memory leak? I'll very appreciate it if you can help me correct the mistake.

/* Copyright 2017 linkedList.c */
#include <stdlib.h>
#include <stdio.h>
#include <cs50.h>
#include <ctype.h>

int LENGTH = 0;

// Linked list structure
typedef struct llnode {
  int val;
  struct llnode* node;
} llnode;

// Delete an entire linkedList
void delete(llnode* head);

// Insert a node (val) at the beginning of a linked list
// Understand why passing a pointer to the head and why front?
void insert(llnode** head, int n);


int main(void) {
  llnode* list = malloc(sizeof(llnode));
  if (list == NULL)
    return -1;

  insert(&list, 2);

  delete(list);

  return 0;
}


/**
 * Delete an entire linkedList
 */
void delete(llnode* head) {
  // temp is used for traversing
  llnode* temp = head;

  /* Nullify every node */
  while (temp != NULL) {
    temp = temp -> node;  // Traverse to next node
    free(head);  // Free memory of previous node
    head = temp;  // Update list to next node
  }
  free(temp);
}

/**
 * Insert a node with integer n at the beginning of a linked list
 *
 * llnode** head: a pointer that points to a pointer variable of
 * a linked list.
 *
 * int n: insert an integer value an in a node
 *
 * return: an updated linked list
 */
void insert(llnode** head, int n) {
  /* Declare another node for insertion */
  llnode* temp = malloc(sizeof(llnode));
  if (temp == NULL) {
    printf("Not enough space can be allocated.\n");
    return;
  }

  /* Track length of the list*/
  LENGTH++;

  /* Assign node values */
  temp -> val = n;
  // The first-ever node points to NULL; afterwards, points to next node
  if (LENGTH == 1)
    temp -> node = NULL;
  else
    temp -> node = *head;

  /* Reassign head */
  *head = temp;
}

The valgrind report is as follows:

==6394== Memcheck, a memory error detector
==6394== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==6394== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==6394== Command: ./question --leak-check=full
==6394== 
==6394== 
==6394== HEAP SUMMARY:
==6394==     in use at exit: 16 bytes in 1 blocks
==6394==   total heap usage: 2 allocs, 1 frees, 32 bytes allocated
==6394== 
==6394== LEAK SUMMARY:
==6394==    definitely lost: 16 bytes in 1 blocks
==6394==    indirectly lost: 0 bytes in 0 blocks
==6394==      possibly lost: 0 bytes in 0 blocks
==6394==    still reachable: 0 bytes in 0 blocks
==6394==         suppressed: 0 bytes in 0 blocks
==6394== Rerun with --leak-check=full to see details of leaked memory
==6394== 
==6394== For counts of detected and suppressed errors, rerun with: -v
==6394== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
1

I'd use llnode* list = NULL; instead of malloc in main. Create nodes only if you have something to store. The content of that non-initialised node is pretty random (both value and next node pointer), so you would not want to use it anyway.

The second free in the delete function is always called with NULL, you should remove it.

In insert, forget about whether LENGTH is 1. Always set the new node's next node to the current head, don't assign NULL pointers there. Here's where you get your leak, you leak the node you malloced in main, which you should not have, see above.

| improve this answer | |
  • Thank you so much for your help!! – tea Jan 28 '17 at 0:30
  • BTW, you can run valgrind with additional options, a few of which are mentioned in the regular output, namely --leak-check=full and -v, but there are more of those. They will produce a lot more output, but might for example print the line where you allocated the block you later lost. – Blauelf Jan 28 '17 at 11:48

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .