Re: new module 'strlcpy'

[Bruno Haible]

Hi Paul,

> Anyway, strlcpy is overkill here, as snprintf does the job just as well 
> here

Not at all. snprintf is not at the right level of abstraction.

There are three levels of abstractions to consider:

  (1) C code which composes a memory region by writing to bytes individually.
  (2) C code which works with strings.
  (3) C code which does memory allocations, formatted output etc.

Here the task is to copy a string to a bounded memory area.

The level (1) - which is embodied by the "strncpy and set NUL byte" approach -
is too low-level, because
  - it is easy to make mistakes at this level (off-by-1),
  - it does not encourage the user to think about truncation and how to
    handle it.

The level (3) - which is embodied by the "snprintf %s" approach =
is too high-level, because
  - it is overkill to parse a format string when all you want to do is
    copy a single string,
  - snprintf can call malloc() and can fail with ENOMEM; these are undesired
    outcomes here.

The level (2) is right for the task, but the standardized functions
(memcpy, strcpy, strncpy) are not up to the job:
  - memcpy because it requires too much preparation code, and is still
    vulnerable to off-by-1 mistakes,
  - strcpy because it may produce buffer overruns,
  - strncpy because it may produce silent truncation.

strlcpy with __warn_unused_result__ is the best solution for this task.

> I'd really rather not promote the use of strlcpy in GNU code, as it is 
> contrary to GNU style.

1) I do not promote "strlcpy" with the flaw of ignoring the return value.
   I promote "strlcpy with __warn_unused_result__", which is an entirely
   different beast, because it will make the users aware of the return
   value and of the silent truncation issue. In some places the users
   will notice that strlcpy does not buy them much, compared to the
   "avoid arbitrary limits"[1] approach, and will switch over to what
   you call "GNU style". In other places, they will insert an abort()
   or assert() to handle the truncation case - which is *better* than
   the strncpy approach.

2) You need to distinguish application code and test code.
   The GNU standards [1] request to avoid arbitrary limits for programs.
   It is completely OK to assume that month names are less than 100 bytes
   long, *in unit tests*. If the same standards were set for test code
   than for application code, it would become even more tedious and
   boring to write unit tests than it already is.

   Probably I should configure the Coverity scan of Gnulib such that
   it puts the test code in a different area, so that we can apply
   different filters and prioritizations to the tests, and so that
   we won't be troubled by "sloppy" style in the tests.

> Plus, I'm not a fan of __warn_unused_result__; in 
> my experience it's too often more trouble than it's worth

We have a module 'ignore-value' that deals with it; so the trouble
you are mentioning must be a trouble in the past, not in the future.

Bruno

[1] https://www.gnu.org/prep/standards/html_node/Semantics.html