7 ms·

Beware of the "correct" solutions proposed here. // OK: correctly copy src char dest[8]; strncpy(dest, src, sizeof(dest) - 1); With a long src, it fails

by tlb 8y ago

Beware of the "correct" solutions proposed here. // OK: correctly copy src char dest[8]; strncpy(dest, src, sizeof(dest) - 1); With a long src, it fails to null-terminate dest. dest[7] will be whatever the contents of uninitialized memory were, so reading dest as a string is likely to run past the end. strlcpy has a better API, though sadly it's not standard on Linux.

majke 8y ago
should be either char dest[8] = {0}; strncpy(dest, src, sizeof(dest) - 1); or char dest[8]; strncpy(dest, src, sizeof(dest) - 1); dest[sizeof(dest)-1] = '\0'; On the same note I always wondered what "snprintf" does. That is - will it ALWAYS zero terminate (given non-NULL buffer, of size > 0)?
- tlb 8y ago
  Yes, snprintf does the right thing.
- BadThink6655321 8y ago
  The first is inefficient. The declaration statement writes eight zeros to dest. strncpy() writes len bytes of zeros to dest, then copies the string, stopping at fewer than len bytes if a NUL is found in the src.
- pascal_cuoq 8y ago
  Yes. “snprintf(dest, sizeof dest, "%s", src);” where dest is a char array almost is the programmer-friend version of strncpy() that does exactly what one would expect, neither more nor less. It always does '\0'-terminate its output as long as it is passed a size >0. The only issue with that idiom is that it is only defined to pass a '\0'-terminated string for src. Although it will only write the specified number of characters, it will read from src until the end of the string, and invoke undefined behavior if src does not point to a well-formed string: https://taas.trust-in-soft.com/tsnippet/t/e0538551 https://taas.trust-in-soft.com/tsnippet/t/e0538551 A mnemonic is that snprintf needs to return the length of the string that would have been output if there had been enough room (not counting the terminating '\0'), so it needs to compute the length of the string pointed by src. A close variant, below, avoids this issue but instead has the problem that the printf star argument is typed as an int, which is narrower than size_t on a typical 64-bit compilation platform. snprintf(dest, "%.*s", (int)(sizeof dest)-1, src);
  - wahern 8y ago
    An annoyance with snprintf is that it has a failure mode, and in particular it can fail with ENOMEM under memory pressure.[1] This isn't merely theoretical as glibc implements snprintf by reusing the stdio machinery--effectively instantiating a temporary FILE structure, buffer, etc. glibc tries to stack allocate these objects using magic constants, but the code is incredibly complex. It's been awhile since I dove into the code, but IIRC snprintf could fail if you try to compose strings longer than the magic internal constants and malloc fails. strlcpy() has the same semantics as snprintf(buf, sizeof buf, "%s", src) but without the failure mode. Because glibc is so stubborn, and rather than trying to wrap snprintf() to abort on OOM, I invariably include this simple implementation in a common header, static size_t aux_strlcpy(char *dst, const char *src, size_t lim) { size_t len, n; len = strlen(src); if (lim > 0) { n = MIN(lim - 1, len); memcpy(dst, src, n); dst[n] = '\0'; } return len; } which is substantially simpler than the canonical version from OpenBSD. A related problem with snprintf is the mixture of signed and unsigned types for communicating object size. Mixing signed and unsigned types is, IME, error prone, so if I have code that uses snprintf more than a few times I usually wrap it in a function that separates the status from the [logical] size return values. People complain that strlcpy is problematic because it similarly communicates status (i.e. truncation) and object size through the same channel. But 1) it's not as nearly as error prone as mixing signed and unsigned types and 2) idiomatic use of strlcpy is easy to code and mentally parse and I've never felt the urge to wrap strlcpy in a helper routine. If truncation is always a failure than I'll simply use a two-line routine that returns an error code. But more often than not I rely on silent truncation. IMO, C code shouldn't be doing complex string operations using native C strings[2], and where it does make sense to use C strings it's usually things like configuration values where garbage in is garbage out; if an overlong property name is truncated it's no different than if it was misspelled. (People seriously overestimate the utility of supporting dynamic object sizes everywhere, and underestimate the inherent complexity it causes, which is significant even in low-level languages like C++ or Rust that make it safer and more convenient.) [1] Linux's overcommit doesn't save you from, e.g., process resource limits. [2] In fact, IMO complex parsing and composition shouldn't even be done with any kind of generic string API. If you get to the point where you're parsing and composing highly structured text, you should be using proper techniques with specialized data types. Writing ad hoc string munging code is error prone and a maintenance nightmare in any language, including scripting languages. But if I must than not only do I avoid C, I avoid any low-level, statically typed language. Scripting languages were literally invented for writing ad hoc string munging code.
- masklinn 8y ago
  Can't you just strncpy sizeof(dest) if you're going to zero-out the last byte of the buffer unconditionally?
  - deleted 8y ago
    [deleted]
  - loeg 8y ago
    Yeah. You've "wasted" an extra byte copy, but that is usually so cheap as to be virtually free.
- MBCook 8y ago
  But just adding the null byte isn’t necessarily safe if you’re dealing with Unicode, right?
  - loeg 8y ago
    Correct. You can't just truncate a utf-8 stream in an arbitrary byte position and expect it to remain well-formed.
wyldfire 8y ago
> strlcpy has a better API, though sadly it's not standard on Linux. I think it's because drepper hates us [1]. [1] http://www.sourceware.org/ml/libc-alpha/2000-08/msg00061.html http://www.sourceware.org/ml/libc-alpha/2000-08/msg00061.htm...
- gowld 8y ago
  > 08 Aug 2000 Any progress since?
  - wyldfire 8y ago
    I think he still hates us ;)
    - loeg 8y ago
      On the plus side, Drepper left Redhat in 2010 and has been out as the glibc DFL since 2012.[0] Amusingly, a request for strlcpy was the very 2nd comment on that LWN article. Unfortunately, glibc still hasn't added it — Roland rejected it again in 2015.[1] Also unfortunately, RH hired Drepper back in 2017. Because they hate their other employees, I guess. Fortunately, no one is eager to give him back the reins of glibc. [0]: https://lwn.net/Articles/488847/ https://lwn.net/Articles/488847/ [1]: https://sourceware.org/ml/libc-alpha/2015-11/msg00086.html https://sourceware.org/ml/libc-alpha/2015-11/msg00086.html
- huhtenberg 8y ago
  > Dammit, it is not safe. It hides bugs in programs. If a string is too long for an allocated memory block the copying must not simply silently stop. This is spot on. There are very few cases when silently truncating a copy of a string is an OK thing to do.
  - wyldfire 8y ago
    I think it's not genuine to describe the behavior of strlcpy() as "silently truncating". strlcpy() is documented as "If the return value is >= dstsize, the output string has been truncated. It is the caller's responsibility to handle this."
bitwize 8y ago
The C11 Annex K functions, strcpy_s et al., have a better API than even strlcpy, and are (an optional) part of the C11 standard. They are what you should be using.
- rurban 8y ago
  Nope. I'm the implementor of this variant, and even the safe variant was designed as unsafe. In my implementation, the safeclib, I made it safe just recently.
dig1 8y ago
Even strncpy() man page warns about this. Correct behavior is: strncpy(buf, str, buflen - 1); if (buflen > 0) buf[buflen - 1] = '\0';
- kevin_thibedeau 8y ago
  It still wastes time zero padding dest when src is shorter.
- unpopular42 8y ago
  If you expect that it's possible for buflen to be 0, you should check before the strncpy call, not after.
  - DSMan195276 8y ago
    Especially since `strncpy` takes a `size_t`, meaning that `buflen - 1` will turn from `-1` to `SIZE_MAX` due to the signed to unsigned conversion. I think OP just slightly misremembered what is on the man page: strncpy(buf, str, n); if (n > 0) buf[n - 1]= '\0'; Which still checks `n` after calling `strncpy`, like you mentioned, but doesn't have the underflow problem.
    - veli_joza 8y ago
      If my count is good, this is correction of correction of correction of original article's correction. And average developer is expected to get it right the first time? It's a miracle anything built in C actually works.
      - deleted 8y ago
        [deleted]
      - orwin 8y ago
        Yes. An average developper with knowledge in C will. Honestly, around my 3rd year i created a lib containing a struct for strings (well, two, and a couple functions working with them) and i don't think i ever used an strX function again. I was pretty proud of the "string for already allocated buffer" part of this lib tbh. Anyway, i'll never take C for a four-hour coding challenge, but for bigger, non-web project, C is really awesome, thank to BLAS and to the fact that most languages implement mostly painless C ffi.
nebulous1 8y ago
He's assuming that dest is zero-initialized, which I believe it will be as written. However, he should mention that because there will be plenty of similar cases where dest will not be initialized. edit: As asveikau notes below, this is not the case
- asveikau 8y ago
  > He's assuming that dest is zero-initialized, which I believe it will be as written. Given this snippet, incorrect. Stack allocated buffers aren't zeroed out as in the above declaration.
  - nebulous1 8y ago
    Well, then that's pretty bad!
- docker_up 8y ago
  No, only global variables are zero-initialized. Neither stack variables nor malloced memory will be zero-initialized. OP should have memset to 0 or calloced the memory.
dwc 8y ago
It's worth noting that strlcpy and strlcat are quite small, very stable, and can be brought into any program that needs them. Yes, it's an extra step but not an onerous one. Of course it's nice to autoconf them if practical.

by tlb 8y ago

majke 8y ago

should be either char dest[8] = {0}; strncpy(dest, src, sizeof(dest) - 1); or char dest[8]; strncpy(dest, src, sizeof(dest) - 1); dest[sizeof(dest)-1] = '\0'; On the same note I always wondered what "snprintf" does. That is - will it ALWAYS zero terminate (given non-NULL buffer, of size > 0)?

tlb 8y ago
Yes, snprintf does the right thing.
BadThink6655321 8y ago
The first is inefficient. The declaration statement writes eight zeros to dest. strncpy() writes len bytes of zeros to dest, then copies the string, stopping at fewer than len bytes if a NUL is found in the src.
pascal_cuoq 8y ago
Yes. “snprintf(dest, sizeof dest, "%s", src);” where dest is a char array almost is the programmer-friend version of strncpy() that does exactly what one would expect, neither more nor less. It always does '\0'-terminate its output as long as it is passed a size >0. The only issue with that idiom is that it is only defined to pass a '\0'-terminated string for src. Although it will only write the specified number of characters, it will read from src until the end of the string, and invoke undefined behavior if src does not point to a well-formed string: https://taas.trust-in-soft.com/tsnippet/t/e0538551 https://taas.trust-in-soft.com/tsnippet/t/e0538551 A mnemonic is that snprintf needs to return the length of the string that would have been output if there had been enough room (not counting the terminating '\0'), so it needs to compute the length of the string pointed by src. A close variant, below, avoids this issue but instead has the problem that the printf star argument is typed as an int, which is narrower than size_t on a typical 64-bit compilation platform. snprintf(dest, "%.*s", (int)(sizeof dest)-1, src);
- wahern 8y ago
  An annoyance with snprintf is that it has a failure mode, and in particular it can fail with ENOMEM under memory pressure.[1] This isn't merely theoretical as glibc implements snprintf by reusing the stdio machinery--effectively instantiating a temporary FILE structure, buffer, etc. glibc tries to stack allocate these objects using magic constants, but the code is incredibly complex. It's been awhile since I dove into the code, but IIRC snprintf could fail if you try to compose strings longer than the magic internal constants and malloc fails. strlcpy() has the same semantics as snprintf(buf, sizeof buf, "%s", src) but without the failure mode. Because glibc is so stubborn, and rather than trying to wrap snprintf() to abort on OOM, I invariably include this simple implementation in a common header, static size_t aux_strlcpy(char *dst, const char *src, size_t lim) { size_t len, n; len = strlen(src); if (lim > 0) { n = MIN(lim - 1, len); memcpy(dst, src, n); dst[n] = '\0'; } return len; } which is substantially simpler than the canonical version from OpenBSD. A related problem with snprintf is the mixture of signed and unsigned types for communicating object size. Mixing signed and unsigned types is, IME, error prone, so if I have code that uses snprintf more than a few times I usually wrap it in a function that separates the status from the [logical] size return values. People complain that strlcpy is problematic because it similarly communicates status (i.e. truncation) and object size through the same channel. But 1) it's not as nearly as error prone as mixing signed and unsigned types and 2) idiomatic use of strlcpy is easy to code and mentally parse and I've never felt the urge to wrap strlcpy in a helper routine. If truncation is always a failure than I'll simply use a two-line routine that returns an error code. But more often than not I rely on silent truncation. IMO, C code shouldn't be doing complex string operations using native C strings[2], and where it does make sense to use C strings it's usually things like configuration values where garbage in is garbage out; if an overlong property name is truncated it's no different than if it was misspelled. (People seriously overestimate the utility of supporting dynamic object sizes everywhere, and underestimate the inherent complexity it causes, which is significant even in low-level languages like C++ or Rust that make it safer and more convenient.) [1] Linux's overcommit doesn't save you from, e.g., process resource limits. [2] In fact, IMO complex parsing and composition shouldn't even be done with any kind of generic string API. If you get to the point where you're parsing and composing highly structured text, you should be using proper techniques with specialized data types. Writing ad hoc string munging code is error prone and a maintenance nightmare in any language, including scripting languages. But if I must than not only do I avoid C, I avoid any low-level, statically typed language. Scripting languages were literally invented for writing ad hoc string munging code.
masklinn 8y ago
Can't you just strncpy sizeof(dest) if you're going to zero-out the last byte of the buffer unconditionally?
- deleted 8y ago
  [deleted]
- loeg 8y ago
  Yeah. You've "wasted" an extra byte copy, but that is usually so cheap as to be virtually free.
MBCook 8y ago
But just adding the null byte isn’t necessarily safe if you’re dealing with Unicode, right?
- loeg 8y ago
  Correct. You can't just truncate a utf-8 stream in an arbitrary byte position and expect it to remain well-formed.

wyldfire 8y ago

> strlcpy has a better API, though sadly it's not standard on Linux. I think it's because drepper hates us [1]. [1] http://www.sourceware.org/ml/libc-alpha/2000-08/msg00061.html http://www.sourceware.org/ml/libc-alpha/2000-08/msg00061.htm...

gowld 8y ago
> 08 Aug 2000 Any progress since?
- wyldfire 8y ago
  I think he still hates us ;)
  - loeg 8y ago
    On the plus side, Drepper left Redhat in 2010 and has been out as the glibc DFL since 2012.[0] Amusingly, a request for strlcpy was the very 2nd comment on that LWN article. Unfortunately, glibc still hasn't added it — Roland rejected it again in 2015.[1] Also unfortunately, RH hired Drepper back in 2017. Because they hate their other employees, I guess. Fortunately, no one is eager to give him back the reins of glibc. [0]: https://lwn.net/Articles/488847/ https://lwn.net/Articles/488847/ [1]: https://sourceware.org/ml/libc-alpha/2015-11/msg00086.html https://sourceware.org/ml/libc-alpha/2015-11/msg00086.html
huhtenberg 8y ago
> Dammit, it is not safe. It hides bugs in programs. If a string is too long for an allocated memory block the copying must not simply silently stop. This is spot on. There are very few cases when silently truncating a copy of a string is an OK thing to do.
- wyldfire 8y ago
  I think it's not genuine to describe the behavior of strlcpy() as "silently truncating". strlcpy() is documented as "If the return value is >= dstsize, the output string has been truncated. It is the caller's responsibility to handle this."

bitwize 8y ago

The C11 Annex K functions, strcpy_s et al., have a better API than even strlcpy, and are (an optional) part of the C11 standard. They are what you should be using.

rurban 8y ago
Nope. I'm the implementor of this variant, and even the safe variant was designed as unsafe. In my implementation, the safeclib, I made it safe just recently.

dig1 8y ago

Even strncpy() man page warns about this. Correct behavior is: strncpy(buf, str, buflen - 1); if (buflen > 0) buf[buflen - 1] = '\0';

kevin_thibedeau 8y ago
It still wastes time zero padding dest when src is shorter.
unpopular42 8y ago
If you expect that it's possible for buflen to be 0, you should check before the strncpy call, not after.
- DSMan195276 8y ago
  Especially since `strncpy` takes a `size_t`, meaning that `buflen - 1` will turn from `-1` to `SIZE_MAX` due to the signed to unsigned conversion. I think OP just slightly misremembered what is on the man page: strncpy(buf, str, n); if (n > 0) buf[n - 1]= '\0'; Which still checks `n` after calling `strncpy`, like you mentioned, but doesn't have the underflow problem.
  - veli_joza 8y ago
    If my count is good, this is correction of correction of correction of original article's correction. And average developer is expected to get it right the first time? It's a miracle anything built in C actually works.
    - deleted 8y ago
      [deleted]
    - orwin 8y ago
      Yes. An average developper with knowledge in C will. Honestly, around my 3rd year i created a lib containing a struct for strings (well, two, and a couple functions working with them) and i don't think i ever used an strX function again. I was pretty proud of the "string for already allocated buffer" part of this lib tbh. Anyway, i'll never take C for a four-hour coding challenge, but for bigger, non-web project, C is really awesome, thank to BLAS and to the fact that most languages implement mostly painless C ffi.

nebulous1 8y ago

He's assuming that dest is zero-initialized, which I believe it will be as written. However, he should mention that because there will be plenty of similar cases where dest will not be initialized. edit: As asveikau notes below, this is not the case

asveikau 8y ago
> He's assuming that dest is zero-initialized, which I believe it will be as written. Given this snippet, incorrect. Stack allocated buffers aren't zeroed out as in the above declaration.
- nebulous1 8y ago
  Well, then that's pretty bad!
docker_up 8y ago
No, only global variables are zero-initialized. Neither stack variables nor malloced memory will be zero-initialized. OP should have memset to 0 or calloced the memory.

dwc 8y ago

It's worth noting that strlcpy and strlcat are quite small, very stable, and can be brought into any program that needs them. Yes, it's an extra step but not an onerous one. Of course it's nice to autoconf them if practical.