6 ms·

It would be interesting to see the rationale behind these bans, and what the suggested alternatives are. Some are obvious, like `strcpy`, but I can't remember w

It would be interesting to see the rationale behind these bans, and what the suggested alternatives are. Some are obvious, like `strcpy`, but I can't remember what the problem with `sprintf` or the time functions are. If you are doing something like `sprintf(buffer, "%f, %f", a, b)`, yes it is tricky to choose the size of buffer frugally, but if you replace that by `ftoa` and constructing the string by hand, you are likely to introduce more bugs. Edit: as pointed out in another post, you can do git blame to see the rationale for each ban, quite interesing.

dahfizz 6y ago
This was my reaction as well. Banning strncpy just encourages haphazard manual copying.
- asdfasgasdgasdg 6y ago
  I think you're meant to use snprintf instead. It would be great to see documentation on the alternatives!
- smasher164 6y ago
  From the commit message: If you're thinking about using it, consider instead: - strlcpy() if you really just need a truncated but NUL-terminated string (we provide a compat version, so it's always available) - xsnprintf() if you're sure that what you're copying should fit - strbuf or xstrfmt() if you need to handle arbitrary-length heap-allocated strings
  - nwmcsween 6y ago
    strlcpy is safer but effectively running strlen(src) every call is a good wtf
- azurezyq 6y ago
  maybe this https://github.com/git/git/blob/master/strbuf.h https://github.com/git/git/blob/master/strbuf.h ?
- ben_bai 6y ago
  strlcpy is the safe way, that is used by git.
- deleted 6y ago
  [deleted]
- syncsynchalt 6y ago
  strncpy doesn't do what you think it does (it is not analogous to strncat). strncpy does not terminate strings on overflow. In C terms, it is not actually a string function and shouldn't be named with `str`. snprintf or nul-plus-strncat do what you want, but snprintf has portability problems on overflow. Most projects I've been on rely on strlcpy (with a polyfill implementation where not available).
  - dahfizz 6y ago
    strnlen may be surprising the first time you see it not null terminate, sure. But if you use the n version of all the string functions (which you should anyway) then it's safe.
monocasa 6y ago
snprintf will always terminate the string, and won't overflow the buffer.
Aanok 6y ago
The trouble with printf-family functions is their variadic nature. If the arguments don't match the format string, you can wreak all sorts of havoc. A fun exercise you can do is put a "%s" in the format string, omit the string argument and see what happens to the stack.
- anyfoo 6y ago
  That's however relatively easy to verify programmatically, and indeed any recent compiler will complain about that. I'd say the usual trap is rather the size of the target buffer, because that requires bigger static analysis guns. (I'm ignoring things like "%n", because then you're playing with fire already.)
  - Gibbon1 6y ago
    I think the big three C compilers have pragma's that you can tag printf/scanf with that will cause the compiler to verify the argument list.
    - xxpor 6y ago
      __attribute__((format(printf, x, y))) for GCC: https://gcc.gnu.org/onlinedocs/gcc-3.2/gcc/Function-Attributes.html https://gcc.gnu.org/onlinedocs/gcc-3.2/gcc/Function-Attribut... There's options other than printf too.
- danaliv 6y ago
  There's that, but with sprintf/vsprintf specifically, there's no way to keep it from storing characters past the end of your buffer. For example: char buf[2]; sprintf(buf, "%d", n); This will happily write to buf[2] and beyond if n is negative or greater than 9.
SloopJon 6y ago
sprintf() warnings have gotten pretty sophisticated these days. I discovered GCC's -Wformat-overflow the other day. It complained that the buffer for a date string wasn't big enough; e.g., sprintf(buf, "%04d-%02u-%02u", year, month, day), where year, month, and day are 16-bit shorts, and buf was probably eleven or twelve bytes. It may actually be a bug that I got the warning, because the range of each input was checked, and I think the compiler is supposed to be smart enough to remember that.

dahfizz 6y ago

This was my reaction as well. Banning strncpy just encourages haphazard manual copying.

asdfasgasdgasdg 6y ago
I think you're meant to use snprintf instead. It would be great to see documentation on the alternatives!
smasher164 6y ago
From the commit message: If you're thinking about using it, consider instead: - strlcpy() if you really just need a truncated but NUL-terminated string (we provide a compat version, so it's always available) - xsnprintf() if you're sure that what you're copying should fit - strbuf or xstrfmt() if you need to handle arbitrary-length heap-allocated strings
- nwmcsween 6y ago
  strlcpy is safer but effectively running strlen(src) every call is a good wtf
azurezyq 6y ago
maybe this https://github.com/git/git/blob/master/strbuf.h https://github.com/git/git/blob/master/strbuf.h ?
ben_bai 6y ago
strlcpy is the safe way, that is used by git.
deleted 6y ago
[deleted]
syncsynchalt 6y ago
strncpy doesn't do what you think it does (it is not analogous to strncat). strncpy does not terminate strings on overflow. In C terms, it is not actually a string function and shouldn't be named with `str`. snprintf or nul-plus-strncat do what you want, but snprintf has portability problems on overflow. Most projects I've been on rely on strlcpy (with a polyfill implementation where not available).
- dahfizz 6y ago
  strnlen may be surprising the first time you see it not null terminate, sure. But if you use the n version of all the string functions (which you should anyway) then it's safe.

monocasa 6y ago

snprintf will always terminate the string, and won't overflow the buffer.

Aanok 6y ago

The trouble with printf-family functions is their variadic nature. If the arguments don't match the format string, you can wreak all sorts of havoc. A fun exercise you can do is put a "%s" in the format string, omit the string argument and see what happens to the stack.

anyfoo 6y ago
That's however relatively easy to verify programmatically, and indeed any recent compiler will complain about that. I'd say the usual trap is rather the size of the target buffer, because that requires bigger static analysis guns. (I'm ignoring things like "%n", because then you're playing with fire already.)
- Gibbon1 6y ago
  I think the big three C compilers have pragma's that you can tag printf/scanf with that will cause the compiler to verify the argument list.
  - xxpor 6y ago
    __attribute__((format(printf, x, y))) for GCC: https://gcc.gnu.org/onlinedocs/gcc-3.2/gcc/Function-Attributes.html https://gcc.gnu.org/onlinedocs/gcc-3.2/gcc/Function-Attribut... There's options other than printf too.
danaliv 6y ago
There's that, but with sprintf/vsprintf specifically, there's no way to keep it from storing characters past the end of your buffer. For example: char buf[2]; sprintf(buf, "%d", n); This will happily write to buf[2] and beyond if n is negative or greater than 9.

SloopJon 6y ago

sprintf() warnings have gotten pretty sophisticated these days. I discovered GCC's -Wformat-overflow the other day. It complained that the buffer for a date string wasn't big enough; e.g., sprintf(buf, "%04d-%02u-%02u", year, month, day), where year, month, and day are 16-bit shorts, and buf was probably eleven or twelve bytes. It may actually be a bug that I got the warning, because the range of each input was checked, and I think the compiler is supposed to be smart enough to remember that.