What does this while line do?

Question

My assignment is to fix Larry Wall's original patch code so that it compiles in ANSI C and the debug it. However, I don't really understand what the code in the savestr function does well enough to fix it.

char *
savestr(s)
register char *s;
{
    register char  *rv,
                   *t;

    t = s;
    while (*t++)
    rv = malloc((MEM) (t - s));
    if (rv == NULL)
        fatal ("patch: out of memory (savestr)\n");
    t = rv;
    while (*t++ = *s++);
    return rv;
}

My understanding is that savestr takes a string argument and returns a string. The function creates two strings, rv and t. and then assigns t to be s. After that line, I'm completely lost. Can someone please explain to me what the code is doing? I especially don't understand how the while loops are moving along, and I have no idea what (t-s) does.

Thank you for any guidance you may have for me.

Answer 1

char *
savestr(s)
register char *s;

This is an old K&R style declaration/definition.

---

The register keyword too, is completely obsolete these days. It was intended as an optimization hint to the compiler. But it is no longer required, except in some rare cases where compiler optimizations are not possible.

---

t = s;
while (*t++);
rv = malloc((MEM) (t - s));

t is assigned the value of s, and is incremented until it reaches the '\0' byte. t - s then gives the difference between the two pointers, which is equivalent to strlen (s) + 1, and then allocates memory for rv, much like strdup().

---

t = rv;

This assigns the pointer value of rv to t, such that they both now point to the same memory location. At the end of the function, only rv will be pointing to this location.

while (*t++ = *s++);

gets parsed as:

while ((*(t++) = *(s++)) != '\0');

In an expression, the post increment operator returns the old value of its operand. So this first copies the character pointed to by s to the character pointed to by t, and then increment both the pointers to point to the their next respective characters. In a loop, this copies the contents of the memory region pointed to by s to the memory region pointed to by t.

---

The mistake is pretty obvious, and has already been pointed out. So I will leave it for OP to figure out.

---

The modern version of savestr looks like this:

char *
savebuf (char const *s, size_t size)
{
  char *rv;

  if (! size)
    return NULL;

  rv = malloc (size);

  if (! rv)
    {
      if (! using_plan_a)
    xalloc_die ();
    }
  else
    memcpy (rv, s, size);

  return rv;
}

char *
savestr (char const *s)
{
  return savebuf (s, strlen (s) + 1);
}

void
xalloc_die (void)
{
  fatal ("out of memory");
}

Notice the ANSI style function declaration/definition and the lack of register keyword. The length is passed by the caller, and the second while loop has been replaced with memcpy(). The macro MEM, which originally evaluated to (unsigned), and the pointer t have been eliminated.

Answer 2

t - s is size (difference between two pointers) and with the statements t = s and while(*t++) implement strlen(s) + 1.

while (*t++ = *s++) copies the value * pointed to by s to the string pointed to by s, and increment both pointers. Strings are terminated by a \0, which is false, so loop will terminate when the \0 has been copied.

All in this implements strdup().

The register keyword probably doesn't do anything on a current compiler. Also, I don't think the (MEM) cast from ptrdiff_t, which is a "signed integer type" (6.5.5), to size_t that malloc() expects is required.