The ups and downs of strlcpy() [LWN.net]

The ups and downs of strlcpy()

Posted Jul 25, 2012 19:13 UTC (Wed) by bronson (subscriber, #4806)
In reply to: The ups and downs of strlcpy() by mmeehan
Parent article: The ups and downs of strlcpy()

Agreed. On top of that, I don't think anybody's saying that strlcpy must always abort, just that it should be safe by default. If you don't want safety, no problem, turn it off and and see what you get.

It would be nice if safety was always the default. Alas, libc (as standardized) only started thinking this way relatively recently.

(Log in to post comments)

The ups and downs of strlcpy()

Posted Jul 25, 2012 20:58 UTC (Wed) by smurf (subscriber, #17840) [Link]

Aborting is not "safe". Aborting is one of at least five ways to handle this particular error. Whether that is 'safe' depends on the context, i.e. your definition of that word.

The fundamental point is that you cannot know beforehand whether looking at the string twice is a [performance] problem, whether truncating (with or without fixing incomplete UTF-8 codes) is better than not starting to fill the buffer in the first place, whether calling abort() is a good idea (I'd say that if you are a library, it almost never is), whether to return something negative or the new length or the source length or …, and a host of related questions, all of which do not lend themselves to consensus answers. As this discussion shows quite clearly, IMHO.

My point is that, with the sole exception of leaving the destination buffer undisturbed when the source won't fit, any of the aforementioned behaviors can be implemented with a reasonably-trivial O(1) wrapper around strlcpy(). Therefore, keeping strlcpy() out of libc is … kindof stupid. Again, IMHO.

Instead, people are told to use strncpy(). Which they'll do incorrectly. Let's face it, running off the end of a string into la-la land is always worse than truncating it.

The ups and downs of strlcpy()

Posted Jul 26, 2012 1:28 UTC (Thu) by nybble41 (subscriber, #55106) [Link]

Here's a suggestion (only partly sarcastic):

typedef size_t (*strxcpy_handler_t)(char *dst, const char *src, size_t size, void *data);

size_t strxcpy(char *dst, const char *src, size_t size, strxcpy_handler_t overflow_fn, void *overflow_data)
{
  char *p;
  const char *q;

  for (p = dst, q = src; *q; ++p, ++q)
  {
    if ((p - dst) >= size)
    {
      return overflow_fn(dst, src, size, overflow_data);
    }

    *p = *q;
  }

  /* get here only if strlen(src) < size */
  *p++ = '\0';
  return (p - dst);
}

size_t strxcpy_truncate(char *dst, const char *src, size_t size, void *data)
{
  if (size <= 0) abort();
  dst[size - 1] = '\0';
  return size + strlen(src + size);
}

size_t strxcpy_abort(char *dst, const char *src, size_t size, void *data)
{
  abort();
  return size;
}

if (strxcpy(dst, src, dst_size, strxcpy_truncate, NULL) >= dst_size) ...;
(void)strxcpy(dst, src, dst_size, strxcpy_abort, NULL);
(void)strxcpy(dst, src, dst_size, strxcpy_subst, "(input too long)");
/* ... */

The ups and downs of strlcpy()

Posted Jul 26, 2012 8:53 UTC (Thu) by renox (subscriber, #23785) [Link]

I'm not sure what is the point of the wrapper function, calling directly strxcpy_abort, strxcpy_truncate, etc would be simpler..

That said, one size doesn't fit all so having different function is reasonable, the biggest issue is that there is no sane default behaviour..

The ups and downs of strlcpy()

Posted Jul 26, 2012 16:27 UTC (Thu) by nybble41 (subscriber, #55106) [Link]

The strxcpy function isn't just a wrapper; it does all of the real work. The strxcpy_abort, strxcpy_truncate functions only run when an overflow condition is detected. This allows you to substitute your own preferred method of error-handling.

This is actually rather similar to the way exceptions are handled in Common Lisp or Scheme programs, except that the Lisp version would use dynamic variables rather than explicit arguments for the handler code, which results in less cluttered code.

(define (default-error-handler error-value) (abort))
(define current-error-handler (make-parameter default-error-handler))

(define (do-something)
  (... (if (ok? var) var ((current-error-handler) var)) ... ))

; aborts on error
(do-something)

; evaluates to #t on success, or #f on error
(let/cc return
  (parameterize ([current-error-handler (lambda _ (return #f))])
    (do-something)
    #t)

; uses "value" in place of var on error
(parameterize ([current-error-handler (lambda _ value)])
  (do-something))

Scheme-style parameters are attached to the current continuation, meaning that they're not only thread-safe, but that the bindings only affect the inner dynamic scope of the (parameterize) form, even in exceptional cases such as non-local return (like the middle example above) and even re-entry into a dynamic scope which was previously exited.