Thread: Troubles re-allocating a struct pointer in a different function.

  1. #1
    Registered User
    Join Date
    Feb 2014
    Posts
    30

    Troubles re-allocating a struct pointer in a different function.

    I'm writing a program that will automatically ban ip addresses that don't follow the proper procedure of displaying a webpage (ie. ip address xxx.xxx.xxx.xxx downloaded index.html but didn't download the background image).


    This code is drastically reduced, but still generates my problem.


    Code:
    // Declare includes.
    #include <stdio.h>
    #include <libgen.h>
    #include <stdlib.h>
    #include <string.h>
    #include <errno.h>
    
    // Declare defines.
    #define ADDY_SIZE        16
    #define DATA_SIZE       500
    #define LOG_FILE          "access.log"
    #define IP_ALLOW_FILE     "ip_allow_list"
    #define IP_BLOCK_FILE     "ip_block_list"
    
    // Declare structs.
    struct _ip
    {
        char addy[ADDY_SIZE];
    };
    
    // Declare function prototypes.
    int add_file (char *file, struct _ip *array, int count);
    int find_marker (FILE *fp);
    char * pad_ip (char *data);
    
    // Declare global variables.
    char *program_name;
    
    int main (int argc, char *argv[])
    {
    // Declare variables.
        int marker = 0;
        int count_block = 0;
        int count_allow = 0;
        int allow_flag = 0;
        FILE *fp_in = NULL;
        char *ptr1 = NULL;
        char temp2[ADDY_SIZE] = {0};
        char data[DATA_SIZE + 1] = {0};
        struct _ip *temp = NULL;
        struct _ip *ips_block = NULL;
        struct _ip *ips_allow = NULL;
    
    // Get program name for error reporting.
        program_name = basename(argv[0]);
    
    // Check for correct number of arguments.
        if(argc != 1)
        {
            fprintf(stderr, "usage: %s\n", program_name);
            exit(EXIT_FAILURE);
        }
    
    // Open input file.
        if((fp_in = fopen(LOG_FILE, "r")) == NULL)
        {
            fprintf(stderr, "%s error: fopen(1) failed (%s) (%s)\n", program_name, LOG_FILE, strerror(errno));
            exit(EXIT_FAILURE);
        }
    
    // Find the marker in the log file.
        marker = find_marker(fp_in);
    
    // Read in all data.
        while(fgets(data, DATA_SIZE, fp_in) != NULL)
        {
    // Reset flag.
            allow_flag = 0;
    
    // Check if ip address is actual user.
            if(strstr(data, "background.png"))
                allow_flag++;
    
    // Find the end of the ip address.
            if((ptr1 = strchr(data, ' ')) == NULL)
            {
                fprintf(stderr, "%s error: strchr failed ( )\n", program_name);
                exit(EXIT_FAILURE);
            }
    
    // Terminate ip address.
            ptr1[0] = '\0';
    
    // Do not process local requests.
            if(data[0] == ':' || strstr(data, "192.168.2.") != NULL)
                continue;
    
    // Increase block record count.
            count_block++;
    
    // Allocate memory for new record.
            if((temp = realloc(ips_block, count_block * sizeof(struct _ip))) == NULL)
            {
                free(ips_block);
                fprintf(stderr, "%s error: realloc(1) failed\n", program_name);
                exit(EXIT_FAILURE);
            }
    
    // Switch pointer.
            ips_block = temp;
    
    // Pad ip address with zeros for sorting.
            strcpy(temp2, pad_ip(data));
    
    // Store the padded results.
            strcpy(ips_block[count_block - 1].addy, temp2);
    
    // Test if address is safe.
            if(allow_flag == 1)
            {
    // Increase allow record count.
                count_allow++;
    
    // Allocate memory for new record.
                if((temp = realloc(ips_allow, count_allow * sizeof(struct _ip))) == NULL)
                {
                    free(ips_allow);
                    fprintf(stderr, "%s error: realloc(2) failed\n", program_name);
                    exit(EXIT_FAILURE);
                }
    
    // Switch pointer.
                ips_allow = temp;
    
    // Pad ip address with zeros for sorting and store results.
                strcpy(ips_allow[count_allow - 1].addy, temp2);
            }
    
    // Reset flag.
            allow_flag = 0;
        }
    
    // Add the allow/block file to the allow/block list.
    //    count_allow = add_file(IP_ALLOW_FILE, ips_allow, count_allow);
    
    // Free allocated memory.
        free(ips_allow);
        free(ips_block);
    
    // Close all file pointers.
        fclose(fp_in);
    
    // Exit gracefully.
        exit(EXIT_SUCCESS);
    }
    
    char * pad_ip (char *data)
    {
    // Declare variables.
        int len = 0;
        char *ptr1 = NULL;
        char *ptr2 = NULL;
        static char ip[16] = {0};
    
    // Get ip address length.
        len = strlen(data);
    
    // Check if ip address is real.
        if(len < 7 || len > 15 || strchr(data, '.') == NULL)
        {
            fprintf(stderr, "%s: pad_ip error: ip test failed (%s)\n", program_name, data);
            exit(EXIT_FAILURE);
        }
    
    // Reset variable.
        ip[0] = '\0';
    
    // Makes looping easier.
        ptr1 = data;
    
    // Loop through all segments.
        while((ptr2 = strchr(ptr1, '.')) != NULL)
        {
    // Terminate segment.
            ptr2[0] = '\0';
    
    // Get segment length.
            len = strlen(ptr1);
    
    // Pad with zeros for sorting.
            if(len == 1)
                strcat(ip, "00");
            else if(len == 2)
                strcat(ip, "0");
    
    // Print segment.
            strcat(ip, ptr1);
            strcat(ip, ".");
    
    // Advance point to next segemnt.
            ptr1 = ptr2 + 1;
        }
    
    // Get segment length.
        len = strlen(ptr1);
    
    // Pad with zeros for sorting.
        if(len == 1)
            strcat(ip, "00");
        else if(len == 2)
            strcat(ip, "0");
    
    // Print segment.
        strcat(ip, ptr1);
        return(ip);
    }
    
    int find_marker (FILE *fp)
    {
    // Declare variables.
        int marker = 0;
        char data[DATA_SIZE + 1] = {0};
    
    // Find the line containing the marker.
        while(data[0] != '-')
        {
            if(fgets(data, DATA_SIZE, fp) == NULL)
            {
                fprintf(stderr, "%s: find_marker error: fgets failed\n", program_name);
                exit(EXIT_FAILURE);
            }
    
    // Advance counter.
            marker++;
        }
    
    // Return marker line number.
        return(marker);
    }
    
    int add_file (char *file, struct _ip *array, int count)
    {
    // Declare variables.
        FILE *fp = NULL;
        struct _ip *temp = NULL;
        char data[DATA_SIZE + 1] = {0};
    
    // Open file.
        if((fp = fopen(file, "r")) == NULL)
        {
            fprintf(stderr, "%s: add_file error: fopen failed (%s) (%s)\n", program_name, file, strerror(errno));
            exit(EXIT_FAILURE);
        }
    
    // Add all records from the file to the struct.
        while(fgets(data, DATA_SIZE, fp) != NULL)
        {
    // Remove trailing newline.
            data[strlen(data) - 1] = '\0';
    
    // Increase the counter.
            count++;
    
    // Allocate memory for new record.
            if((temp = realloc(array, count * sizeof(struct _ip))) == NULL)
            {
                free(array);
                fprintf(stderr, "%s: add_file error: realloc failed\n", program_name);
                exit(EXIT_FAILURE);
            }
    
    // Switch pointer.
            array = temp;
    
    // Copy data to struct.
            strcpy(array[count - 1].addy, pad_ip(data));
        }
    
    // Close the file pointer.
        fclose(fp);
    
    // Return the new count total.
        return(count);
    }
    This code runs fine under valgrind. But if you uncomment:
    Code:
    count_allow = add_file(IP_ALLOW_FILE, ips_allow, count_allow);
    Then I get the follow from valgrind:
    Code:
    ==4164== Memcheck, a memory error detector
    ==4164== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
    ==4164== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
    ==4164== Command: ./gtt
    ==4164== 
    ==4164== 
    ==4164== HEAP SUMMARY:
    ==4164==     in use at exit: 48 bytes in 1 blocks
    ==4164==   total heap usage: 16 allocs, 15 frees, 10,112 bytes allocated
    ==4164== 
    ==4164== 48 bytes in 1 blocks are definitely lost in loss record 1 of 1
    ==4164==    at 0x4C2D13F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==4164==    by 0x4010A2: add_file (reduced2.c:255)
    ==4164==    by 0x400CB8: main (reduced2.c:134)
    ==4164== 
    ==4164== LEAK SUMMARY:
    ==4164==    definitely lost: 48 bytes in 1 blocks
    ==4164==    indirectly lost: 0 bytes in 0 blocks
    ==4164==      possibly lost: 0 bytes in 0 blocks
    ==4164==    still reachable: 0 bytes in 0 blocks
    ==4164==         suppressed: 0 bytes in 0 blocks
    ==4164== 
    ==4164== For counts of detected and suppressed errors, rerun with: -v
    ==4164== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
    I haven't had problems with realloc before. I know if I initialize the pointers to NULL, that both realloc and free shouldn't complain.

    I make sure I realloc to another pointer (temp2) first, as a realloc error isn't guaranteed to free the original pointer. If successful, I move the temp2 pointer to the original struct.

    Valgrind appears to be pointing at the realloc in the function add_file. But why an error? I do the same thing I did in the main function for realloc.

  2. #2
    Registered User
    Join Date
    May 2009
    Posts
    3,675
    Code:
    int add_file (char *file, struct _ip *array, int count)
    You are changing the value of array in rare cases inside add_file.
    Therefore, you need to return the changed value or you get lost memory or bad frees.

    Tim S.
    "...a computer is a stupid machine with the ability to do incredibly smart things, while computer programmers are smart people with the ability to do incredibly stupid things. They are,in short, a perfect match.." Bill Bryson

  3. #3
    misoturbutc Hodor's Avatar
    Join Date
    Nov 2013
    Posts
    1,457
    Quote Originally Posted by stahta01 View Post
    Code:
    int add_file (char *file, struct _ip *array, int count)
    You are changing the value of array in rare cases inside add_file.
    Therefore, you need to return the changed value or you get lost memory or bad frees.

    Tim S.
    More accurately I think the function is supposed to be realloc()ating memory for 'array' which would then be later deallocated in main (or wherever the rest of the stuff is deallocated). The add_file function should be getting a pointer to a pointer (**array) rather than just *array I believe. I've only glanced at the code dump though.

  4. #4
    Registered User
    Join Date
    Feb 2014
    Posts
    30
    Quote Originally Posted by Hodor View Post
    More accurately I think the function is supposed to be realloc()ating memory for 'array' which would then be later deallocated in main (or wherever the rest of the stuff is deallocated). The add_file function should be getting a pointer to a pointer (**array) rather than just *array I believe. I've only glanced at the code dump though.
    Ty for your replies.

    I changed the call to add_file like so:
    Code:
    count_allow = add_file(IP_ALLOW_FILE, &ips_allow, count_allow);
    And all references to 'array' in the function add_file I changed to '*array' (and after changing the struct '.' to '->'). But it won't compile. It give the following error:
    Code:
    reduced2.c: In function ‘add_file’:
    reduced2.c:275:16: warning: passing argument 1 of ‘strcpy’ makes pointer from integer without a cast [-Wint-conversion]
             strcpy(*array[count - 1]->addy, pad_ip(data));
                    ^
    In file included from reduced2.c:5:0:
    /usr/include/string.h:126:14: note: expected ‘char * restrict’ but argument is of type ‘char’
     extern char *strcpy (char *__restrict __dest, const char *__restrict __src)
                  ^~~~~~
    What I don't understand is that before I decided to add data from another file, I was only doing it from the original 'access.log' file. Read the whole file in, and sorted it. The call to the sort function doesn't raise the struct it sends like you suggest. Yet, the struct is able to be sorted. Why can I sort a struct without raising it when sending it to a sort function, but when I try to re-allocate with the same type of call, I need to raise it? How can the sort function make permanent changes to the struct without the struct being raised?

    I hope I'm making sense, because this problem is not making sense to me.

  5. #5
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    37,493
    > strcpy(*array[count - 1]->addy, pad_ip(data));
    Now you need to write
    strcpy((*array)[count - 1]->addy, pad_ip(data));
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Function with struct pointer
    By flegmik in forum C Programming
    Replies: 7
    Last Post: 11-22-2011, 10:03 AM
  2. Replies: 17
    Last Post: 07-06-2011, 11:44 AM
  3. Using function pointer within a struct
    By Hannibal2010 in forum C Programming
    Replies: 10
    Last Post: 06-19-2011, 10:49 PM
  4. function pointer in a struct
    By dayalsoap in forum C Programming
    Replies: 3
    Last Post: 04-02-2011, 08:57 PM
  5. Function and pointer troubles
    By ulillillia in forum C Programming
    Replies: 7
    Last Post: 04-25-2009, 04:25 PM

Tags for this Thread