4 ms·

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.
    - DSMan195276 8y ago
      I would add that while I do normally use regular C strings rather then my own library, I still can't remember the last time I used any of the random `str*` functions. I don't do much string composition anyway, but the little I do is almost always using `snprintf` or variants (Which have a non-braindead API, unlike `strncpy`). Tons easier to read and much more effective then a bunch of `strcpy`, `strcat` and such. The only thing I wish the standard library would have is an `asprintf` which would return an allocated buffer. I've written my own, but the easy version utilizing regular `sprintf` requires calling `sprintf` twice (Once to get the length, and once to actually put the string in the allocated buffer). A regular supported version would be much more efficient.
      - deleted 8y ago
        [deleted]
- jwilk 8y ago
  dig1 remembered it right. This bug was introduced in man-pages 3.62: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=3f89ae779fce17911a920267fbba1b6f17042e2b https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/...
  - DSMan195276 8y ago
    Huh, well that's embarrassing. It looks like it's still in there too. I googled the man page but I guess I saw an older version. I might submit a patch if I find time. It should really be something like this: if (n > 0) { strncpy(buf, str, n - 1); buf[n - 1] = '\0'; } Of course, the error isn't super huge considering if you're using a zero-length buffer as a null-terminated string you're going to have other problems. But they still do check for 0, so IMO it's worth correcting.
    - jwilk 8y ago
      Matthew Kilgore submitted patch to fix this: https://marc.info/?l=linux-man&m=152834469929748 https://marc.info/?l=linux-man&m=152834469929748
      - dig1 8y ago
        Huh, I blindly pasted example assuming man pages are always right. Double check next time :D
      - DSMan195276 8y ago
        Yes, that would be me :)

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.
  - DSMan195276 8y ago
    I would add that while I do normally use regular C strings rather then my own library, I still can't remember the last time I used any of the random `str*` functions. I don't do much string composition anyway, but the little I do is almost always using `snprintf` or variants (Which have a non-braindead API, unlike `strncpy`). Tons easier to read and much more effective then a bunch of `strcpy`, `strcat` and such. The only thing I wish the standard library would have is an `asprintf` which would return an allocated buffer. I've written my own, but the easy version utilizing regular `sprintf` requires calling `sprintf` twice (Once to get the length, and once to actually put the string in the allocated buffer). A regular supported version would be much more efficient.
    - deleted 8y ago
      [deleted]
jwilk 8y ago
dig1 remembered it right. This bug was introduced in man-pages 3.62: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=3f89ae779fce17911a920267fbba1b6f17042e2b https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/...
- DSMan195276 8y ago
  Huh, well that's embarrassing. It looks like it's still in there too. I googled the man page but I guess I saw an older version. I might submit a patch if I find time. It should really be something like this: if (n > 0) { strncpy(buf, str, n - 1); buf[n - 1] = '\0'; } Of course, the error isn't super huge considering if you're using a zero-length buffer as a null-terminated string you're going to have other problems. But they still do check for 0, so IMO it's worth correcting.
  - jwilk 8y ago
    Matthew Kilgore submitted patch to fix this: https://marc.info/?l=linux-man&m=152834469929748 https://marc.info/?l=linux-man&m=152834469929748
    - dig1 8y ago
      Huh, I blindly pasted example assuming man pages are always right. Double check next time :D
    - DSMan195276 8y ago
      Yes, that would be me :)