Konkret
Konkret

Reputation: 1021

How to make an actual copy of a struct (not a pointer)

This program uses an external file known as island.txt, and builds a linked list out of every line items. The problem is, as of this writing, I cannot copy the struct properly, it merely does a pointer to the current struct which is created. Can you guys get this program to print "Result: Success!!!"?

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
/**
 * The data format of an island defined within a linked-list format
*/
typedef struct island {
    char * name;
    char * opens;
    char * closes;
    struct island * next;
} island;
/**
 *  The island create method 
*/
island * create ( char * name ){
    island * country = malloc(sizeof(island)); // Allocate a memory on the heap for the country
    country->name = strdup(name); // Allocate a memory on the heap for the name
    country->opens = "6:00 AM";
    country->closes = "11:00 PM";
    country->next = NULL;  
    free(country);
    return country;
}
/**
 * Automatically link islands togerther forming a link list on-new-line event
*/
void autoLink( char * country_name, char * file_name ){
    FILE * islands; // Define islands as a file  
    islands = fopen(file_name, "r");
    island * start_country = NULL; // An island variable to record the 1st item of the linked list
    island * country = NULL; // The next country item of an island
    island * previously_created_country = NULL;
    if(islands == NULL){
        perror("Error opening the file");
    } 
    else{
        char previous_country_name[80]; // The previous island name
        fgets(previous_country_name, 80, islands);
        while(fgets(country_name, 80, islands) != NULL){
            country = create(country_name);
            if(start_country == NULL){ // When starting the loop the next country is the start (1st country)
                start_country = country;  
            }
            else{  
                previously_created_country = create(previous_country_name);               
                previously_created_country->next = country;
                printf("\nThe current name is: %s", country_name);
                printf("Country name inside struct: %s", country->name);
                printf("The previously created country name inside the struct: %s", previously_created_country->name);
                printf("The previous name is: %s", previous_country_name);
                if(strcmp(country_name, country->name)){
                    printf("*****Result: Country name error!!!*****\n");
                }
                if(strcmp(previously_created_country->name, previous_country_name)){
                    printf("*****Result: Previous Country name error!!!*****\n");
                }
                if(!(strcmp(country_name, country->name) && strcmp(country_name, country->name))){
                    printf("*****Result: Success!!!*****\n");
                }
            }
            strcpy(previous_country_name, country_name);                    
        }
    }
}
/**
 * Starting point
*/
int main () {
    char name[80]; // The name of a given island
    char file_name[30] = "islands.txt";
    autoLink(name, file_name);
    return 0;
}

Here's the islands.txt file content:

Haiti
Anguilla
Antigua and Barbuda
Aruba
The Bahamas
Barbados
Bonaire
British Virgin Islands
Cayman Islands
Cuba
Curaçao
Dominica
Dominican Republic
Federal Dependencies of Venezuela
Grenada
Guadeloupe
Jamaica
Martinique
Montserrat
Navassa Island
Nueva Esparta
Puerto Rico
Saba
San Andrés and Providencia
Saint Barthélemy
Saint Kitts and Nevis
Saint Lucia
Saint Martin
Saint Vincent and the Grenadines
Sint Eustatius
Sint Maarten
Trinidad and Tobago
Turks and Caicos Islands
United States Virgin Islands

Upvotes: 0

Views: 61

Answers (1)

dbush
dbush

Reputation: 223917

In your create function:

island * create ( char * name ){
    island * country = malloc(sizeof(island)); // Allocate a memory on the heap for the country
    country->name = strdup(name); // Allocate a memory on the heap for the name
    country->opens = "6:00 AM";
    country->closes = "11:00 PM";
    country->next = NULL;  
    free(country);      // <---- BAD
    return country;
}

After you finish populating your object, you free it and return the freed pointer. That memory is no longer value as a result of beeing freed, and attempting to use a pointer to freed memory invokes undefined behavior.

Get rid of the free in this function.

Upvotes: 2

Related Questions