Thread: 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.

Originally Posted by stahta01

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.

Originally Posted by Hodor

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.