ANSI C printf gives wrong output when called second time

Question

I am having strange problems with printf and freeing memory allocations. (As you guys always ask for full code I am posting it. Hope you don't mind the length of it.)

I am writing this onto a start-up code. So I am not allowed to change structs or functions.

i.e. void system_free(tm_type_ptr tm) is given to me and not allowed to change it.

csv files are posted right at the end. Now problems:

1) A printf in a for loop prints value of a struct elements. Which prints out correct values. A duplicate copy of that printf where placed right after the first one gives me messed up values.

printfs are in purchase_ticket()

the output I get is:

First printf

  5 = 10
 10 =  5
 20 =  8
 50 =  2
100 = 20
200 =  8

Second printf

  5 = 10
 10 =  5
 20 =  8
 50 =  2
100 = 20
200 = 56

This output is the best I can get. Only 200s value is different here. Sometimes I get 32232 kind of values as well.

2) system_free() gives me below error. Probably I don't know how to free the memory or where/when to free.

*** glibc detected *** ./tm: double free or corruption (out): 0x00007fff812582b0 ***
======= Backtrace: =========
/lib64/libc.so.6[0x38816760e6]
/lib64/libc.so.6[0x3881678c13]
./tm[0x4018e6]
./tm[0x400bde]
/lib64/libc.so.6(__libc_start_main+0xfd)[0x388161ecdd]
./tm[0x400a29]
======= Memory map: ========
00400000-00403000 r-xp 00000000 fd:01 264655                             /home/assignment/tm
00602000-00603000 rw-p 00002000 fd:01 264655                             /home/assignment/tm
011f7000-01218000 rw-p 00000000 00:00 0                                  [heap]
3471200000-3471216000 r-xp 00000000 fd:00 131168                         /lib64/libgcc_s-4.4.7-20120601.so.1
3471216000-3471415000 ---p 00016000 fd:00 131168                         /lib64/libgcc_s-4.4.7-20120601.so.1
3471415000-3471416000 rw-p 00015000 fd:00 131168                         /lib64/libgcc_s-4.4.7-20120601.so.1
3880e00000-3880e20000 r-xp 00000000 fd:00 136224                         /lib64/ld-2.12.so
388101f000-3881020000 r--p 0001f000 fd:00 136224                         /lib64/ld-2.12.so
3881020000-3881021000 rw-p 00020000 fd:00 136224                         /lib64/ld-2.12.so
3881021000-3881022000 rw-p 00000000 00:00 0
3881600000-388178a000 r-xp 00000000 fd:00 136225                         /lib64/libc-2.12.so
388178a000-3881989000 ---p 0018a000 fd:00 136225                         /lib64/libc-2.12.so
3881989000-388198d000 r--p 00189000 fd:00 136225                         /lib64/libc-2.12.so
388198d000-388198e000 rw-p 0018d000 fd:00 136225                         /lib64/libc-2.12.so
388198e000-3881993000 rw-p 00000000 00:00 0
3881e00000-3881e83000 r-xp 00000000 fd:00 136235                         /lib64/libm-2.12.so
3881e83000-3882082000 ---p 00083000 fd:00 136235                         /lib64/libm-2.12.so
3882082000-3882083000 r--p 00082000 fd:00 136235                         /lib64/libm-2.12.so
3882083000-3882084000 rw-p 00083000 fd:00 136235                         /lib64/libm-2.12.so
7fa7870b9000-7fa7870bc000 rw-p 00000000 00:00 0
7fa7870c5000-7fa7870c9000 rw-p 00000000 00:00 0
7fff81245000-7fff8125a000 rw-p 00000000 00:00 0                          [stack]
7fff81336000-7fff81337000 r-xp 00000000 00:00 0                          [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]
./t.sh: line 2: 25356 Aborted                 (core dumped) ./tm

---

MY CODE:

*********
  tm.h
*********

#include <stdio.h>

#include <stdlib.h>
#include <string.h>
#include <assert.h>
#ifndef TM_H
#define TM_H

#define NUM_COINS 6
#define TICKET_NAME_LEN 40
#define TICKET_ZONE_LEN 10
#define TICKET_TYPE_LEN 10
#define DEFAULT_STOCK_LEVEL 10
#define DEFAULT_COINS_COUNT 50
#define NUM_ARGS 3
#define MAX_LINE_SIZE 1024
#define DELIMITER ","

struct stock_data 
{
    char ticket_name[TICKET_NAME_LEN+1];
    char ticket_type;
    char ticket_zone[TICKET_ZONE_LEN+1];
    unsigned int ticket_price;
    unsigned int stock_level;
};

typedef struct stock_node 
{
    struct stock_data * data;
    struct stock_node * next_node;
} stock_node;

struct stock_list
{
    stock_node * head_stock;
    unsigned int num_stock_items;
};


enum coin_types {
    FIVE_CENTS=5,
    TEN_CENTS=10,
    TWENTY_CENTS=20,
    FIFTY_CENTS=50,
    ONE_DOLLAR=100,
    TWO_DOLLARS=200
};

struct coin {
    enum coin_types denomination;
    unsigned count;
};


typedef struct tm * tm_type_ptr;

typedef struct stock_list * stock_list_ptr;

typedef struct coin * coin_list_ptr;

typedef struct tm {
    coin_list_ptr coins;
    stock_list_ptr stock;
} tm_type;

#endif

---

#include "tm.h"

int main() {

    tm_type tm;
    tm_type * tm_ptr;
    tm_ptr = &tm;

    system_init(&tm);

    load_data(&tm, "stock.csv", "coins.csv");

    purchase_ticket(&tm);

    system_free(tm_ptr);

    exit(0);
}


void purchase_ticket(tm_type * tm) {

   struct stock_data user_input, compare;

   int stock_count = 0, x;
   int xxx[NUM_COINS] = { 5,10,20,50,100,200 };

   stock_count = tm->stock->num_stock_items;

   system("clear");

   for (x = 0; x < NUM_COINS; x++) { printf("\n%3d = %2d", xxx[x], tm->coins[x].count); }
   printf("\n\n");

   for (x = 0; x < NUM_COINS; x++) { printf("\n%3d = %2d", xxx[x], tm->coins[x].count); }
   printf("\n\n");   

   while (getchar() != '\n'){}
   return;
}


void load_data(tm_type * tm, char * stockfile, char * coinsfile)
{

   char temp_line[MAX_LINE_SIZE];
   char *token;
   int i, count = 0, number_coin, turn = 1;

   stock_node * snode = NULL;

   struct coin temp_coin[6];

   FILE *stock_file = fopen( stockfile, "r" );
   FILE *coins_file = fopen( coinsfile, "r" );

   while (fgets(temp_line, MAX_LINE_SIZE, stock_file) != NULL) {

      token = strtok (temp_line, DELIMITER);
      count++;

      snode = (stock_node *) realloc(snode, count * sizeof(stock_node));
      if (snode == NULL) { abort(); }  

      snode[count - 1].data = (struct stock_data *) calloc(1, sizeof(struct stock_data));

      if (snode[count - 1].data == NULL) { abort(); }   

      i = 1;

      while(token != NULL) {
         switch(i) {
            case 1:
               strcpy(snode[count - 1].data->ticket_name, token);
               break;
            case 2:
               snode[count - 1].data->ticket_type = token[0];
               break;
            case 3:
               strcpy(snode[count - 1].data->ticket_zone, token);
               break;
            case 4:
               snode[count - 1].data->ticket_price = atoi(token);
               break;
            case 5:
               snode[count - 1].data->stock_level = atoi(token);
               break;                                                            
         }
         token = strtok (NULL, DELIMITER);
         i++;
      }
   }

   struct stock_list slist = { snode, count };

   stock_list_ptr slist_ptr = &slist;

   tm->stock = slist_ptr;

   i = 0;

   while (fgets(temp_line, MAX_LINE_SIZE, coins_file) != NULL) {
      token = strtok (temp_line, DELIMITER);
      while(token != NULL) {
         token = strtok (NULL, DELIMITER);
         if(token != NULL) {
            sscanf(token, "%d", &number_coin);
         }

         if (turn == 1) {
            switch(number_coin) {
               case 5:
                  temp_coin[i].denomination = FIVE_CENTS;
                  break;
               case 10:
                  temp_coin[i].denomination = TEN_CENTS;
                  break;
               case 20:
                  temp_coin[i].denomination = TWENTY_CENTS;
                  break;
               case 50:
                  temp_coin[i].denomination = FIFTY_CENTS;
                  break;
               case 100:
                  temp_coin[i].denomination = ONE_DOLLAR;
                  break;
               case 200:
                  temp_coin[i].denomination = TWO_DOLLARS;
                  break;
            }

            turn = 2;

         } else {
            temp_coin[i].count = number_coin;
            turn = 1;
         }
      }

      i++;
   }   

   coin_list_ptr clist_ptr = (coin_list_ptr) malloc(sizeof(coin_list_ptr));
   if (clist_ptr == NULL) { abort(); }  

   clist_ptr = temp_coin;

   tm->coins = clist_ptr;

   fclose(stock_file);
   fclose(coins_file);

   return;
}


void system_free(tm_type_ptr tm) {
    free(tm->stock);
}

void abort () {
   exit(1);
}

---

CSV FILES:

Coins.csv   (<coin>, <coin_count>)
---------
5,10
10,5
20,8
50,2
100,20
200,8


Stock.csv    (<ticketName>,<type>,<zone>,<price>,<stockLevel>)
---------
2 hour,F,1,350,5
Weekly,C,1+2,2370,10
Daily,F,2,700,8
Weekly,F,1,1240,9
Daily,C,1,200,5

Answer 1

Your problem is in this area (might have the same problem other places but this one stuck out at me)

coin_list_ptr clist_ptr = (coin_list_ptr) malloc(sizeof(coin_list_ptr));
if (clist_ptr == NULL) { abort(); }  

clist_ptr = temp_coin;
tm->coins = clist_ptr;

The var 'temp_coin' is an array locally defined in the function (also known as stack allocated). You are setting it to (eventually) to a member of tm which is being returned from the method. This is bad because after the function returns that stack memory is reused. You can't returned memory addresses allocated locally in your function in variable that leave your function. The local variables are stack allocated, but variables being returned need to be heap allocated. If you change the following (in load_data):

struct coin temp_coin[6];

to

struct coin temp_coin = malloc(6 * sizeof(coin));

You will have better results. The only reason the first printf loop worked ok is luck, if the code were a bit different it could have easily printed corrupt values there.

----- follow up from comment ----- In the post you linked you had this code:

snode = (stock_node *) realloc(snode, count * sizeof(stock_node));
struct stock_list slist = { snode, count };
stock_list_ptr slist_ptr = (stock_list_ptr) malloc(sizeof(stock_list_ptr));
slist_ptr = &slist;
tm->stock = slist_ptr;

People were trying to communicate that you don't need to both allocate and initialize. For example:

(1) struct stock_list slist = { snode, count };
(2) stock_list_ptr slist_ptr = (stock_list_ptr) malloc(sizeof(stock_list_ptr));
(3) slist_ptr = &slist;
(4) tm->stock = slist_ptr;

Line 1 allocates space for the struct on the stack. Line two allocates space on the heap for the struct. Line three overwrites the address of your heap allocated memory with your stack allocated memory. Line four assigns your stack allocated memory to you tm->stock. It would be better to do this:

stock_list_ptr slist_ptr = (stock_list_ptr) malloc(sizeof(stock_list)); // note sizeof(stock_list) not stock_list_ptr
if (slist_ptr == NULL) {/* error do something */ }
slist_ptr->head_stock = snode;
slist_ptr->num_stock_items = count;
tm->stock = slist_ptr;

Hope that helps clear it up