CODEWITHSUNDEEP
clinux
Howard Shane
Howard Shane

Reputation: 956

asprintf(): how to free the pointers?

I have code like this: I assigned log twice, is there potential memory leak for the first &log?

char *log = NULL;
asprintf(&log, "Hello: %s", name);
if (known_person== true){
    asprintf(&log, "%s, %s", log, ", my old friend.");
}
free (log);

Upvotes: 9

Views: 2444

Answers (3)

alk
alk

Reputation: 70921

is there [a] [...] memory leak

Definitely yes.

The reference to the memory allocated by the 1st call to aprintf() gets overwritten by the 2nd call to aprintf(), so there is no chance to free() the 1st allocated memory any more, it "leaks".

To fix this introduce a 2nd (temporary) pointer:

char name[] = "C";
char * log = NULL;

{
  char * log_tmp = NULL;

  asprintf(&log_tmp, "Hello: %s", name);
  if (known_person == true)
  {
    asprintf(&log, "%s, %s", log_tmp, ", my old friend.");
    free(log_tmp);
  }
  else
  {
    log = log_tmp;
  }
}

/* Use log. */

free(log);

A different and probably cheaper (faster as some stuff is handle during compilation time, but run-time) approach to this issue would be the following:

#define FORMAT_STR "Hello: %s"
#define FORMAT_SUFFIX_STR ", my old friend."

...

char name[] = "C";
char * log = NULL;

{
  char format[sizeof FORMAT_STR""FORMAT_SUFFIX_STR + 1] = FORMAT_STR;

  if (known_person == true)
  {
    strcat(format, FORMAT_SUFFIX_STR);
  }

  asprintf(&log, format, name);
}

/* Use log. */

free(log);

Adding error checking on system calls is left to reader as an exercise.

A 3rd approach to this only using Standard C functions is:

char name[] = "C";
char * log = NULL;

{
  char * log_tmp = NULL;

  asprintf(&log_tmp, "Hello: %s", name);
  if (known_person == true)
  {
    asprintf(&log, "%s, %s", log_tmp, ", my old friend.");
    free(log_tmp);
  }
  else
  {
    log = log_tmp;
  }
}

/* Use log. */

free(log);

A different and probably cheaper (faster as some stuff is handle during compilation time, but run-time) approach to this issue would be the following:

#define FORMAT_STR "Hello: %s"
#define FORMAT_SUFFIX_STR ", my old friend."

...

char name[] = "C";
char * log = NULL;

{
  char format[sizeof FORMAT_STR""FORMAT_SUFFIX_STR + 1] = FORMAT_STR;

  if (known_person == true)
  {
    strcat(format, FORMAT_SUFFIX_STR);
  }

  {
    int s = snprintf(NULL, 0, format, name);
    if (-1 == s) 
    {
      /* failure */
    }
    else
    {
      log = malloc(s + 1);
      if (NULL == log)
      {
        /* failure */
      }
      else
      {
        if (-1 == sprintf(log, format, name))
        {
          /* failure */
        }
      }
    }
  }
}

free(log);

Upvotes: 2

user3386109
user3386109

Reputation: 34829

Yes, the code will leak, since asprintf neither checks, nor attempts to reuse, the previous pointer. Hence, the memory is simply lost. The best way to avoid the problem in your example would be to rewrite the code as

char *log = NULL;
if (known_person== true)
    asprintf(&log, "Hello: %s, my old friend.", name);
else
    asprintf(&log, "Hello: %s", name);

free (log);

That way, the buffer is allocated once and freed correctly.

Alternatively, you could use two pointers

char *temp = NULL;
asprintf(&temp, "Hello: %s", name);

char *log = NULL;
if (known_person== true) {
    asprintf(&log, "%s, my old friend.", temp);
    free( temp );
}
else {
    log = temp;
}

free (log);

Upvotes: 7

Daniel A. White
Daniel A. White

Reputation: 190935

Yes. Likely asprinf won't hit the same memory place and has no knowledge of the previous calls.

Upvotes: 0

Related Questions