0

Context: Explanation of pointing to same address problem
compile:

gcc \
    -Wpointer-arith -Wendif-labels \
    -Wimplicit-fallthrough -Wmissing-format-attribute  \
    -Wcast-function-type -Wshadow=compatible-local \
    -Wformat-security -Wno-format-truncation -Wno-stringop-truncation \
    main.c &&./a.out dat/test.csv

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include<errno.h>

enum {MAXB = 128, NELEM = 10};

typedef struct data_t{
    char *name;
    int id;
    double balance;
}data_t;

int main(int argc,char **argv)
{
    if(argc < 2){
        printf("No file input.foramt a.out filename. %s\n",argv[1]); 
        exit(EXIT_FAILURE);
    }

    FILE *fp = fopen(argv[1],"r");
    if(fp == NULL){
        printf("file not found\n");
        exit(EXIT_FAILURE);
    }

    char buffer[MAXB] = "";
    const char *delim = ",\n";
    int index = 0;

    data_t **array = malloc(NELEM * sizeof *array);
    if(!array){ /* validate EVERY allocation */
        perror("malloc-array failed\n");
        exit(EXIT_FAILURE);
    }


    /* protect the allocation bounds, you only have NELEM pointers */
    while (index < NELEM && fgets(buffer,sizeof buffer,fp))
    {
        data_t customer;
        size_t len = 0;
        
        char *tmp;          /* temporary pointer to use w/strtok() */
        tmp = strtok(buffer,delim);  /* token for name */
        if(!tmp)                     /* validate token */
        {
            fputs("error: strtok() name.\n",stderr);
            exit(EXIT_FAILURE);
        }
        len = strlen(tmp);               /* get length */
        customer.name = malloc(len+1);   /* allocate for string */
        if(!customer.name)
        {
            perror("malloc-customer.name\n");
            exit(EXIT_FAILURE); 
        }
        memcpy(customer.name,tmp, len +1);  /* copy tmp to .name */

        if(!(tmp = strtok(NULL,delim)))     /* token & validations */
        {
            fprintf(stderr,"error: strtok() -id.\n");
            exit(EXIT_FAILURE);
        }
        
        /* MINIMAL conversion validation with sscanf() */
        if(sscanf(tmp,"%d",&customer.id) != 1){
            fprintf(stderr,"error: invalid integer value for line[%d]\n",index+1);
            continue;
            /* exit(EXIT_FAILURE); */
        }

        if(!(tmp = strtok(NULL,delim))){
            fprintf(stderr,"\nerror: strtok()-balance shuld have more line\n");
            continue;
            // exit(EXIT_FAILURE); 
        }

        if(sscanf(tmp,"%lf", &customer.balance) != 1){
            fprintf(stderr,"error: invalid float value for line[%d]\n", index+1);
             /* exit(EXIT_FAILURE); */
            continue;
        }

        array[index] = malloc(sizeof *array[index]);    /* allocate struct */
        if(!array[index]){                              /* validate!! */
            perror("malloc-array[index]");
            exit(EXIT_FAILURE);
        }
        *array[index] = customer;                       /* assign struct */
        index += 1;

    }
    fclose(fp);
    
    int i;
    printf("\nname\tid\t balance\n\n");
    for(i = 0; i< index; i++)
        printf("%s \t%d\t%f\n",array[i]->name,array[i]->id,array[i]->balance);

    for(i = 0; i< index; i++)
    {
        if(array[i]->name)
            free(array[i]->name);       /* free allocated block for name */
        if(array[i])
            free(array[i]);             /* free allocated array index */
    }

    free(array);
    exit(EXIT_FAILURE);
}

cat dat/test.csv returns

name,id,balance
test1,11,1.2334
test2,12,1.133
test3,13,1.3334
test4,14,1.4334

command valgrind -s --leak-check=full ./a.out dat/test.csv output:

==494197== Memcheck, a memory error detector
==494197== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==494197== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==494197== Command: ./a.out dat/test.csv
==494197==
error: invalid integer value for line[1]

name    id       balance

test1   11      1.233400
test2   12      1.133000
test3   13      1.333400
test4   14      1.433400

==494197== Memcheck, a memory error detector
==494197== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==494197== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==494197== Command: ./a.out dat/test.csv
==494197==
error: invalid integer value for line[1]

name    id       balance

test1   11      1.233400
test2   12      1.133000
test3   13      1.333400
test4   14      1.433400
==494197==
==494197== HEAP SUMMARY:
==494197==     in use at exit: 5 bytes in 1 blocks
==494197==   total heap usage: 13 allocs, 12 frees, 5,797 bytes allocated
==494197==
==494197== 5 bytes in 1 blocks are definitely lost in loss record 1 of 1
==494197==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==494197==    by 0x10951D: main (in /home/jian/helloc/a.out)
==494197==
==494197== LEAK SUMMARY:
==494197==    definitely lost: 5 bytes in 1 blocks
==494197==    indirectly lost: 0 bytes in 0 blocks
==494197==      possibly lost: 0 bytes in 0 blocks
==494197==    still reachable: 0 bytes in 0 blocks
==494197==         suppressed: 0 bytes in 0 blocks
==494197==
==494197== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
jian
  • 4,119
  • 1
  • 17
  • 32
  • Worse than memory leak: `if(array[i]]->name) {} if(array[i]) {}` – imagine what happens if second test fails – then you are dereferencing a null pointer in first test! You need to swap this here anyway: `if(array[i]){ if(array[i]->name) { ... } ... }`. Though `free` accepts a null pointer as well, so you actually would not need to test for the name: `if(array[i]) { free(array[i]->name); free(array[i]); }`... – Aconcagua Dec 24 '22 at 10:52
  • 2
    In the first iteration there's `customer.name = malloc(len+1);` and then `fprintf(stderr,"error: invalid integer value for line[%d]\n",index+1); continue;` which leaks that allocation. You need `free(customer.name);` before the `continue`. – Retired Ninja Dec 24 '22 at 11:01
  • 1
    @RetiredNinja yes. you are right. – jian Dec 24 '22 at 11:14
  • https://stackoverflow.com/questions/4007268/what-exactly-is-meant-by-de-referencing-a-null-pointer @Aconcagua you probably right. – jian Dec 24 '22 at 11:32
  • After looking at your code once again: Actually you don't need *any* of these checks: Your `index` only is incremented after both calls on `malloc` have succeeded, so up to `index` all array fields are populated anyway (unless you still modify `index` elsewhere...). – Aconcagua Dec 24 '22 at 11:35
  • You could reduce the overhead for all of these allocations and programme complexity tremendously by only dynamically allocating the names, by the way: You have a fix number of array entries (`NELEM`) and a fix – and small – size of objects anyway. So why not simply have `data_t array[NELEM]`? All that remains to free then is the names... – Aconcagua Dec 24 '22 at 11:40

0 Answers0