0

Note: I did call this function and free it main but valgrind still shows error. This code basically takes in a singly linked-list with two data coeff and exp. This is basically converting a polynomial store in a linked list converted to readable string. I looking to have it dynamic allocated.

char *Poly_to_string(const Polynomial *p)
{
    char *x = malloc(1);
    int size;

    while (p != NULL)
    {
        if((p->exp != 0) && (p->exp != 1))
        {
            size = asprintf(&x, "%s%dx^%d + ", x, p->coeff, p->exp);
            if (size == -1)
            {
                exit(-1);
            }
        }   
        else if(p->exp == 1)
        {
            size = asprintf(&x, "%s%dx + ", x, p->coeff);
            if (size == -1)
            {
                exit(-1);
            }
        }
        else if(!p->exp)
        {
            size = asprintf(&x, "%s%d + ", x, p->coeff);
            if (size == -1)
            {
                exit(-1);
            }
        }
        p = p->next;
    }
    x[strlen(x) - 3] = '\0';
    return x;
}
user14972264
  • 13
  • 4

2 Answers2

1

From the Linux asprintf() man page (bolding mine):

DESCRIPTION

The functions asprintf() and vasprintf() are analogs of sprintf(3) and vsprintf(3), except that they allocate a string large enough to hold the output including the terminating null byte ('\0'), and return a pointer to it via the first argument. This pointer should be passed to free(3) to release the allocated storage when it is no longer needed.

RETURN VALUE

When successful, these functions return the number of bytes printed, just like sprintf(3). If memory allocation wasn't possible, or some other error occurs, these functions will return -1, and the contents of strp are undefined.

This line is wrong:

char *x = malloc(1);

It should just be

char *x;

because if asprintf() works, it will overwrite the contents of x and cause the memory allocated in char *x = malloc(1); to be leaked.

EDIT

The looping also needs to be addressed, as you're trying to grow the string:

char *Poly_to_string(const Polynomial *p)
{
    // start with an empty string that can be free()'d
    // (if you don't have strdup() use malloc() and strcpy())
    char *x = strdup("");
    int size;

    while (p != NULL)
    {
        // save the old malloc()'d value so it can be free()'d
        char *oldValue = x;

        if((p->exp != 0) && (p->exp != 1))
        {
            size = asprintf(&x, "%s%dx^%d + ", x, p->coeff, p->exp);
            if (size == -1)
            {
                exit(-1);
            }
        }   
        else if(p->exp == 1)
        {
            size = asprintf(&x, "%s%dx + ", x, p->coeff);
            if (size == -1)
            {
                exit(-1);
            }
        }
        else if(!p->exp)
        {
            size = asprintf(&x, "%s%d + ", x, p->coeff);
            if (size == -1)
            {
                exit(-1);
            }
        }

        // free() the old value
        free(oldValue);
        p = p->next;
    }
    x[strlen(x) - 3] = '\0';
    return x;
}

There are other ways to do this without the initial char *x = strdup(""); but the code then becomes a lot more complex.

Andrew Henle
  • 32,625
  • 3
  • 24
  • 56
  • thanks. but the memory leak is still there – user14972264 Jan 09 '21 at 14:11
  • You're also looping over multiple `Polynomial *p` since it's a linked list. You need to retain a copy of the old pointer in `x` and free it after each iteration. I missed that on my first look at your code. – Andrew Henle Jan 09 '21 at 14:18
  • Thank you. Your help solved a couple of my problems. Much appreciated. There is still a couple of leaks, but progress has been made! – user14972264 Jan 09 '21 at 14:43
  • I wondering if the x in between each statement is causing the main leak problems – user14972264 Jan 09 '21 at 14:44
  • I don't see how, but this is just one part of your entire bit of code, I assume. Valgrind should be telling you where the leaked memory was allocated from. – Andrew Henle Jan 09 '21 at 14:50
  • I'm sorry it is all gone >.< I've forgot to recompile and was working on a old batch. Your help was greatly appreciated. Also, I definitely gain a better understanding on dynamic memory allocation. – user14972264 Jan 09 '21 at 15:18
0

You're not deallocating variable x

Hasan Manzak
  • 663
  • 6
  • 14
  • in that regards am i need to free x after I move the contents to another variable? if so I think that's where I lacking knowledge and wisdom, some advice in that regards would help me – user14972264 Jan 09 '21 at 14:07