Péter Lakos
Péter Lakos

Reputation: 28

malloc()/free() in a loop

I have an assignment for a class where I have a text file, bikes.txt. The contents are attributes of bikes like so:

bike_id=16415
bike_station_id=455
bike_status=free

bike_id=6541
bike_station_id=1
bike_status=reserved

bike_id=5
bike_station_id=6451
bike_status=reserved

Right now I'm trying to read all bike_id's, and I have a question:

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

char* getAttribute(const char* attr, const char* line);

int main(int argc, char** argv)
{
    FILE* file = fopen("bikes.txt", "r");
    if (file != NULL)
    {
        char line[256];
        while (fgets(line, sizeof(line), file))
        {
            if (strstr(line, "bike_id=") != NULL)
            {
                char* bikeIdText = getAttribute("bike_id", line);
                printf("\"%s\"", bikeIdText);
                //free(bikeIdText);
                //bikeIdText = NULL;
            }
        }
    }
}

char* getAttribute(const char* attr, const char* line)
{
    int lineLength = strlen(line);
    int attrLength = strlen(attr);
    // +1 because of "="
    char* attrText = malloc(lineLength - attrLength + 1);
    // +2 because of "=" and NEWLINE
    memcpy(attrText, line + attrLength + 1, lineLength - (attrLength + 2));
    return attrText;
}

The above code works. The output is:

"16415""6541""5"

The problem is that - if I'm right - the getAttribute() function will allocate more and more memory which won't get freed. However, if I uncomment the free(bikeIdText); and bikeIdText = NULL; lines in main(), the output shows that the same memory location is used, because the longer values won't get overwritten by shorter ones. The output in this case:

"16415""65415""55415"

How could I solve the problem?

Upvotes: 0

Views: 199

Answers (2)

alk
alk

Reputation: 70901

This

  char* attrText = malloc(lineLength - attrLength + 1);

shall be

  char * attrText = malloc(lineLength - (attrLength + 1));
  attrText[lineLength - (attrLength + 1) - 1] = '\0' ;

or the equivalent

  char * attrText = malloc(lineLength - attrLength - 1);
  attrText[lineLength - attrLength - 2] = '\0' ;

This assumes line to end with one addtional character.

Upvotes: 1

Stasik
Stasik

Reputation: 2614

Last character of the string is not set to '\0' therefore, the "%s" prints more than it should (%s stops output at \0 byte).

Try to malloc one byte more char* attrText = malloc(lineLength - attrLength + 2) and set, attrText[lineLength - attrLength + 1] = '\0' before return.

Upvotes: 1

Related Questions