Dynamic memory allocation + truncating a string issue

Question

Hi. First post here. I've been fooling around with malloc, realloc and free in order to write some basic functions to operate on C strings (char*). I've encountered this weird issue when erasing the last character from a string. I wrote a function with such a prototype:

int string_erase_end (char ** dst, size_t size);

It's supposed to shorten the "dst" string by one character. So far I have come up with this code:

int string_erase_end (char ** dst, size_t size)
{
    size_t s = strlen(*dst) - size;
    char * tmp = NULL;
    if (s < 0) return (-1);
    if (size == 0) return 0;
    tmp = (char*)malloc(s);
    if (tmp == NULL) return (-1);
    strncpy(tmp,*dst,s);
    free(*dst);
    *dst = (char*)malloc(s+1);
    if (*dst == NULL) return (-1);
    strncpy(*dst,tmp,s);
    *dst[s] = '\0';
    free(tmp);
    return 0;
}

In main(), when I truncate strings (yes, I called malloc on them previously), I get strange results. Depending on the number of characters I want to truncate, it either works OK, truncates a wrong number of characters or throws a segmentation fault.

I have no experience with dynamic memory allocation and have always used C++ and its std::string to do all such dirty work, but this time I need to make this work in C. I'd appreciate if someone helped me locate and correct my mistake(s) here. Thanks in advance.

Answer 1

According to your description your function is supposed to erase the last n characters in a string:

/* Assumes passed string is zero terminated... */
void string_erase_last_char(char * src, int num_chars_to_erase)
{
    size_t len = strlen(src);

    if (num_chars_to_erase > len)
    {
        num_chars_to_erase = len;
    }

    src[len - num_chars_to_erase] = '\0';
} 

Answer 2

I don't understand the purpose of the size parameter.

If your strings are initially allocated using malloc(), you should just use realloc() to change their size. That will retain the content automatically, and require fewer operations:

int string_erase_end (char ** dst)
{
  size_t len;
  char *ns;

  if (dst == NULL || *dst == NULL)
   return -1;

  len = strlen(*dst);
  if (len == 0)
    return -1;

  ns = realloc(*dst, len - 1);
  if (ns == NULL)
   return -1;
  ns[len - 1] = '\0';
  *dst = ns;

  return 0;
}

In the "real world", you would generally not change the allocated size for a 1-char truncation; it's too inefficient. You would instead keep track of the string's length and its allocated size separately. That makes it easy for strings to grow; as long as there is allocated space already, it's very fast to append a character.

Also, in C you never need to cast the return value of malloc(); it serves no purpose and can hide bugs so don't do it.

Answer 3

The first strncpy() doesn't put a '\0' at the end of tmp.

Also, you could avoid a double copy: *dst = tmp;