13 ms·

Beware of strncpy and strncat

• 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.
• ebikelaw 8y ago
  C++ basic_string::c_str always returns a null-terminated string. C++ is the solution to numerous C pitfalls.
  • AnimalMuppet 8y ago
    True. But not everyone can use it. If you can, it's (almost always) the right answer.
• eboyjr 8y ago
  Interesting how the "solutions" to the buffer overflow problems don't provide for all of the modern assumptions of programming with strings. I would love to know the history of the development of strncpy.
  • kazinator 8y ago
    I suspect that strncpy was intended for filling in fields in records to be written to files, where you want all the bytes to be clean, with no random junk (potentially sensitive) after the null byte. For instance struct utmp in Unix or something of its ilk.
    • slrz 8y ago
      Exactly. Or the fixed-length directory entries in old Unix file systems. If the file name is exactly 14 chars long, you don't care about NUL termination but if it is any less you want to zero out the remaining bytes. Strncpy is made for that. It's not restricted to writing to disk. When these structures cross the userspace/kernel boundary on a system call, you really don't want to leave uninitialized bytes following the NUL terminator and return them to some user process.
      • deleted 8y ago
        [deleted]
      • wahern 8y ago
        The modern use case for strncpy is for filling in the .sun_path member of struct sockaddr_un. Most people assume that the path needs to be NUL terminated, but the BSD Sockets API actually relies on the declared sockaddr length parameter. It's not superfluous and the kernel will only read .sun_path up to the end of the declared size of the sockaddr structure; it doesn't expect NUL termination though it will obey internal NUL termination. Moreover, the statically declared size of .sun_path in the libc headers doesn't limit the maximum length of the path. On most implementations you can create domain socket paths larger than this. Indeed, when you use an API like getsockname() you normally should check for truncation by comparing the returned sockaddr length with the size of the buffer you passed. Just like with snprintf() and strlcpy(), if the returned logical length is greater than your buffer size the path was truncated. IIRC, not all implementations (or any?) include a NUL byte as part of the length so you can very well end up with a .sun_path that isn't NUL terminated if your buffer only barely fit the path. Likewise if you didn't 0-initialize the path buffer and the actual path was shorter, though IIRC kernels handle this second case differently--some might NUL terminate for good measure if there's space.
• vortico 8y ago
  Don't use `strncpy()` and `strncat()` at all, use snprintf(buffer, buffer_len, "%s Stuff %s", str1, str2); It's safe (C99, C++11) and easily extendible. Format strings are fun! Not the fastest, but if the bottleneck of your program is concatinating strings, just do it manually.
  • Hello71 8y ago
    This is a safe, if slow, alternative for strncpy, but it does not safely replace strncat. The C standard does not define the behavior if the code "sprintf(buf, "%s some further text", buf);" is used.
    • dmitrygr 8y ago
      #define strncat(a, b, maxlen) snprintf(a + strlen(a), maxlen - strlen(a), "%s", b)
      • kazinator 8y ago
        Too many evaluations of a. If you have snprintf, you probably have inline functions. One day someone will do something like strncat(a += previous_delta, ...).
        • dmitrygr 8y ago
          OPTION 1 (c): char* mystrncat(char *a, const char *b, size_t maxlen){ snprintf(a + strlen(a), maxlen - strlen(a), "%s", b); return a; } OPTION 2 (gccism): #define strncat(_a, b, maxlen) ({char* a = _a; snprintf(a + strlen(a), maxlen - strlen(a), "%s", b); _a;})
    • vortico 8y ago
      Right, don't use `buf` as an input and output with `snprintf`. A lot of people use `str(n)cat` and `str(n)cpy` together like char *a; char *b; char *c; strcpy(c, a); strcat(c, b); and I meant that `snprintf` can replace that. But if you actually only have two strings, one with a larger buffer than what it contains, and one to be concatinated, then you can't use `snprintf` like that.
• Hello71 8y ago
  In addition to tlb's point, the article's description of strncat is not correct. > As with strcat(), the resulting string in dest is always null-terminated. > Therefore, the size of dest must be at least strlen(dest)+n+1.
  • jwilk 8y ago
    They wrote: // XXX: after this buf may not be null-terminated strncat(buf, another_buffer, BUF_SIZE - strlen(buf)); This code is indeed bad, but not because the result won't be null-terminated, but because it's an off-by-one buffer overflow. Despite this misunderstanding, the proposed fix is correct: // OK: ensure buf is null-terminated after concatenation strncat(buf, another_buffer, BUF_SIZE - strlen(buf) - 1);
• chris_va 8y ago
  For those similarly curious: """ char * STRNCPY (char s1, const char s2, size_t n) { size_t size = __strnlen (s2, n); if (size != n) memset (s1 + size, '\0', n - size); return memcpy (s1, s2, size); } libc_hidden_builtin_def (strncpy) """ ... which is actually different than how I thought it would be implemented (it ends up with an extra loop to figure out the size of the string). (side note, how does one format code in HN?)
  • kazinator 8y ago
    > how does one format code in HN? Two space indent.
• kazinator 8y ago
  Never memcpy structs, except if you need to ensure that the padding bytes are copied. C has had a = b assignment for structs since long before ANSI C.
• Analemma_ 8y ago
  I'm just shouting into the void here, but why does anyone find it acceptable that C is almost fifty years old– a half-century– and we still have new articles published about the correct way to copy memory. And then, immediately following them, comments in responds to those articles saying the article is wrong and that you should actually do it this other way. Nobody has figured this out in 50 years?
  • legohead 8y ago
    I'm with you. I started with C some ~20 years ago, and you learn very early on about null terminating strings. You are constantly thinking about it for every manipulation you do. This article isn't exactly ground breaking news. It smells more like an amateur just got bit and decided to write up a blog about it.
  • kazinator 8y ago
    A few new people have to learn this stuff. That's not just to maintain legacy code. Computers do not have nicely behaved, safe, garbage collected strings. Someone has to understand the code for how that stuff is bootstrapped, and that code is going to have gunky memory copies in it where a one word mistake will bring down the show.
  • krapp 8y ago
    The problem is - what even is a "string" in C? An arbitrary number of arbitrary bytes that you hope ends in a null, but you'll never know unless you check, and even when you check, do you really know? Was that null really where that string was supposed to end? There's never going to be a single, universally valid and correct way to deal with a "type" that is structurally just a step above random garbage.
  • pjmlp 8y ago
    It runs deep than that. C suffers from a macho culture, where many developers deeply believe that only the others do mistakes. Even Dennis was quite clear that C was not supposed to be used without help from lint (developed in 1979). "Although the first edition of K&R described most of the rules that brought C's type structure to its present form, many programs written in the older, more relaxed style persisted, and so did compilers that tolerated it. To encourage people to pay more attention to the official language rules, to detect legal but suspicious constructions, and to help find interface mismatches undetectable with simple mechanisms for separate compilation, Steve Johnson adapted his pcc compiler to produce lint [Johnson 79b], which scanned a set of files and remarked on dubious constructions." -- https://www.bell-labs.com/usr/dmr/www/chist.html https://www.bell-labs.com/usr/dmr/www/chist.html Guess how many do run lint or any other static analyzers as part of their build.
• TimJYoung 8y ago
  I just don't understand why C hasn't been blessed with a proper string type yet. Object Pascal has had one (actually, several) for decades now and it doesn't hinder the language's ability to handle low-level memory manipulation (you can still manually copy string memory, convert them back/forth into raw Char pointers, etc.), and generally serves to make string handling much, much safer for most applications. It does however, result in slightly more memory consumption for the length/reference count tags and does incur some overhead in the form of compiler-generated reference count checks at the end of functions. But, IMO, the advantages outweigh the disadvantages for general-purpose C programming and you're always free to fall back to the more manual methods of handling character arrays. So, am I missing something and there is some concrete reason why this can't be implemented ?
  • vortico 8y ago
    There are many object oriented string types for C in various independent GitHub repositories, but they're not really interesting for C programmers because they enjoy the simplicity of null-terminated strings and moving data around with `for` loops.
    • TimJYoung 8y ago
      I understand that (one of my favorite books on my shelf is "C Interfaces and Implementations" that shows some of the cool stuff that you can do with any C implementation, including "proper" strings), but they're not something supported in the compiler, which is, unless I'm mistaken, necessary for the reference count checks. My point is that implementing a new string type has zero effect upon existing C programs if they don't use the new string type, so I'm confused as to why it hasn't been done. If a C developer doesn't want to use them, then "no harm, no foul".
      • vortico 8y ago
        Well, then you have two string types. The language will instantly become complicated as we indecisively choose between two different string types each function in our APIs. I see no harm in adding more functions around the string type we already have, but as I mentioned in https://news.ycombinator.com/item?id=17248446 https://news.ycombinator.com/item?id=17248446, `snprintf` is the mother-of-all-string-functions that does everything you need, so not much else is needed.
• nine_k 8y ago
  Probably back in 1970s `(characters, \000)` looked "elegant", and `(counter, characters)`, wasteful. This decision, if not the single most prolific, is likely one of the top 3 sources of security exploits in C code. All for the want of saving a few bytes.
  • asdfman123 8y ago
    What, you're saying that features that made sense for a programming language designed in the 70s don't make sense in 2018?
    • nine_k 8y ago
      Note how it's not a feature of a programming language per se, but rather a detail of implementation of a common data structure. Likely strings not being their own type but being directly represented as `char[]` also looked elegant and commendably parsimonious back in the day. Can't blame people who worked on machines with 128 kilobytes of RAM for that. They likely did not expect that their well-meaning hacks will take over most of the computing world.
      • pjmlp 8y ago
        The people that worked in machines 10 years older than PDP-11 managed to have safer systems languages. Still being sold today as Unisys ClearPath mainframes.
      • berti 8y ago
        Plenty of systems still have less RAM than that (hint: microcontroller). Unsurprisingly they're overwhelmingly C.
        • pjmlp 8y ago
          Those systems also have Basic and Pascal compilers, when one bothers to look around. https://www.mikroe.com/compilers https://www.mikroe.com/compilers Also many of those micro-controllers are more powerful than the computers where we ran CP/M on, where we had plenty of system languages to choose from. They are have become overwhelmingly C due to synergy effects.
• GuB-42 8y ago
  I usually avoid strncpy() and strncat() altogether char buf[256]; size_t sz = strlen(str); if (sz < sizeof (buf)) { memcpy(buf, str, sz + 1); } else { /* error processing */; } Truncation is not as bad as a buffer overflow. However, it is still not correct. You have to properly handle the case. And if truncating is the correct answer, make that explicit. In practice, I almost never use fixed size buffers for strings unless I know the size at compile time. Edit (for strcat): char buf[256] = "blah"; size_t sz1 = strlen(buf); size_t sz2 = strlen(str); if (sz1 + sz2 < sizeof (buf)) { memcpy(buf + sz1, str, sz2 + 1); } else { /* error processing */; }
• docker_up 8y ago
  Not even this works. They forgot to force the last byte as a NULL, which is a classic bug in C. Either that or memset the char array before using it. But what the blog poster did is a pure bug.
  • kazinator 8y ago
    NULL is a pointer constant; a byte is never described as NULL (in the context of C).
• Someone 8y ago
  ”C programmers should use the newer strncpy()” Newer? I thought strncpy dates back to the time Unix filenames were 14 characters, max, adding padding zeroes when needed in some fixed-length kernel structures. That’s also the reason strncpy always writes len bytes; not keeping garbage content in those 14-byte buffers allows the system to use memcmp to compare file names.
• GlitchMr 8y ago
  Worth noting that strncpy doesn't stand for secure string copy or anything like that. Using strncpy for copying strings would be a mistake, even if technically you can do that. Rather, it's a fixed size string copy function. This structure is very rare in regular environments, but they can happen in embedded environments. For instance, if you want to have a string in binary file which is at most 10 bytes, you may want to avoid storing the termination byte when the string is exactly 10 bytes long. For instance, such a structure was used in UNIX to store file names, as they used to be limited to 14 bytes, and storing terminator would be a waste of space.
• hawski 8y ago
  There is no idiomatic way to use strncpy, unless you're running the 7th edition of Unix [0]. [0] https://stackoverflow.com/a/1454071/6561829#6561829 https://stackoverflow.com/a/1454071/6561829#6561829
• jhallenworld 8y ago
  There are many worse string formats than C's NUL terminated: Last byte of string has bit 7 set. First byte of string has length, so strings are limited to 255 bytes in length. First two bytes of string has length, so strings are limited to 65535 bytes in length. Strings are stored in fixed length buffers with space padding to the end. Length prefixed strings are stored in a fixed length buffer, so you are limited to the buffer length. I think this was the case for PL/I "varying" strings. Back in the day, C was better than PASCAL because it had strdup, meaning it had a heap and you could put strings in it. C++ string is mediocre. I solves some problems, but what if: You have very long strings and you are worried about heap fragmentation. So you are better to have something like a linked list of segments each in their own malloc block. But can you extend std::string? Nope, oh well. You want strings to be semipredicates. I mean that strings should be able to have a NULL value, as I can do with C. (return NULL for 'char *'). Can std::string do this? Nope. Can it be extended? Nope.
  • thestoicattack 8y ago
    > You want strings to be semipredicates. I mean that strings should be able to have a NULL value, as I can do with C. A null value different from "emptiness"? Would you be okay with a std::string* or a std::optional<std::string>?
    • jhallenworld 8y ago
      "since C++17", I see.. I'm curious if it uses more space than "char *" Also it's not great because I should be able to pass such a string through functions that expect std::string. A NULL string should act just like an empty string except that you can test it for NULL.
      • thestoicattack 8y ago
        Yes. std::optional<T> allocates the T in-place. Just checked compiler explorer, and on g++ 8.1, sizeof(std::string) == 32, sizeof(std::optional<std::string>) == 40. (of course sizeof(char* ) == 8)
  • 8y ago
• bio_end_io_t 8y ago
  The author should've been wary of strncpy, because his solution is wrong. "If there is no null byte among the first n bytes of src, the string placed in dest will not be null-terminated." So copying sizeof(dest)-1 will not append a NULL byte as the author implies. You'll have to do that manually.
  • mturmon 8y ago
    Article's solution is wrong, but at least the headline is right!
• nneonneo 8y ago
  The advice given in this article is bad. Reducing the length of the copy by one will still fail to null-terminate the string if the source exceeds the destination length. You need to add dst[sizeof(dst)-1] = 0; or memset the array to zero beforehand. You are not guaranteed to have a zero at the end of the input buffer otherwise (neither local variable arrays nor malloc’d arrays are guaranteed to be zeroed). strncpy sucks for a second reason: it writes n bytes no matter what the source is. That means if you write char buffer[16384]; strncpy(buffer, “hello”, sizeof(buffer)-1); buffer[sizeof(buffer)-1] = 0; this will fill 16K of memory with zeroes despite only needing to copy a 6-byte string. strlcpy/strlcat do the right thing (copy up to n-1 bytes and null-terminate); shame they aren’t standardized. In their absence, I suggest snprintf instead: snprintf(buffer, sizeof(buffer), “%s”, src); Because snprintf returns the number of bytes that would be written, it can be used to detect overlong input strings, reallocate the buffer as necessary, and also to implement efficient concatenation. It’s also surprisingly fast in most implementations.
  • blattimwind 8y ago
    > The advice given in this article is bad. Thumbing through previous submissions of this domain with discussion, this seems to be a pattern.
  • laythea 8y ago
    strncpy would have to assume that it was working on strings if that were not the case.....oh wait....
  • beeforpork 8y ago
    Same advice everywhere, very good, my words exactly: use snprintf(). But beware of old MS _snprintf: https://docs.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2012/2ts7cx93(v=vs.110) https://docs.microsoft.com/en-us/previous-versions/visualstu...
  • kdelok 8y ago
    Is there any reason not to use snprintf for almost every application of string manipulation (aside from strduping)?
• professorTuring 8y ago
  This link has nothing different to offer than "man 3 strncpy"... Worse, some examples are invalid.
  • charlchi 8y ago
    People complaining about memory safety in functions whose man pages explicitly state memory safety is not provided...
• beeforpork 8y ago
  Well, this is not very helpful advice, because strncpy(a, b, sizeof(a)) is in no way more safe than strncpy(a, b, sizeof(a)-1), because the latter is not 0-terminated either. And from malloc(), as in the examples, comes no 0-termined buffer, but random garbage memory. What would be safer is to alway 0-terminate the buffer after copying, and using the simplest copy possible: strcpy(a, b, sizeof(a)); a[sizeof(a)-1] = 0; But this is more boilerplate and hence more error-prone. Even safer, use strlcpy() (if available) or snprintf() which both 0-terminate (except under Windows, maybe). (But beware when preparing something for copying from trusted to untrusted: strncpy() clears the rest of the buffer while strlcpy() and snprintf() do not, so you might leak info via uninitialised memory behind the end of the string if you copy out that buffer across a trust boundary. Actually, the authors 'sizeof()-1' solution is less secure in this context.) So, use: snprintf(a, sizeof(a), "%s", b); And don't tell me anything about speed, please. Your main concern with C is not micro optimisations but robustness and avoiding undefined behaviour (and that snprintf() is not too slow). And for multiple concats, use multiple snprintfs(), like so: char *i = a, *e = a + sizeof(a); i += snprintf(i, e-i, "%s", b1); i += snprintf(i, e-i, "%s", b2); i += snprintf(i, e-i, "%s", b3); This is the most concise way I know to write this that works without buffer overflow (your main enemy, even more vile than missing 0-termination), without thinking too much, without writing too much boilerplate, and that is relatively robust against breaking in code restructuring (like, appending more stuff in the middle). The idiom also resembles a bit old style C++ iterators ('i' and 'e'). Oh, and a truncated string is usually not good anyway, be it 0-terminated or not. So you do need to check for that after all that stringing stuff: if (strnlen(a, sizeof(a)) >= sizeof(a)-1) { /* ... error ... */ } Don't miss that '-1' there. Off-by-one is another enemy to know well. And dispite that check handling missing 0-termination, do not be tempted to fall back to strcpy(), because missing 0-termination is bad(tm). Phew! C is bad with strings. The above resembles C++ iterators ('i' and 'e') and works fine with any good snprintf implementation (i.e., probably not under Windows). And do not copy structs with memcpy, just assign them! memcpy() is for arrays only. This is not going to go away, is it?
  • nneonneo 8y ago
    Mostly good advice, but the multiple `snprintf` example is wrong - it contains a buffer overflow. snprintf returns the number of characters that would be written, so when you do i += snprintf(i, e-i, "%s", b1); i would end up past e if b1 is overlong. Then in the next line i += snprintf(i, e-i, "%s", b2); e-i is negative, but snprintf takes a size_t so this will overflow badly. The best solution, AFAICT, is the following: int res; res = snprintf(i, e-i, "%s%s%s", b1, b2, b3); if(res >= e-i) { /* handle overflow */ return -1; } else { i += res; } /* subsequent snprintf's here */
• mlthoughts2018 8y ago
  I like learning about these caveats, but I have been asked tricky stuff like this in interviews before with gets() and the like. As a person who interviews other people, I find that it's waaay more valuable that someone is generally aware that they should watch out for this class of pitfalls than that they know any specifics about a given function. I've met people who basically had memorized the description of this phenomenon for gets(), but then their preferred solution was just to replace it with fgets() but then they don't know about checking for newlines or have any thoughts on what to do when individual lines are too long. I'd much rather hire someone who says to herself, "Oh, I need to read some characters from an input source using C. RED ALERT! Let me really research the specifics here." Instead of someone who thinks, "Oh, I need to read some characters from an input source using C. Good thing I memorized that trivia about gets() and can totally solve this in the best way immediately with the highest upvoted Stack Overflow solution of fgets() that I didn't bother to deeply grok." I find that when interviews are geared towards puzzle solving or esoteric trivia, the people who do well are mostly of the second type (the ones I wouldn't want to hire). Whereas someone of the first type might flounder around and struggle in a 20-minute programming task to process strings in C, directly because that person cares more about having a bigger picture point of view of what's actually going on rather than esoteric memorization of specific function signatures and usage mechanics. In other words, if I gave some kind of C string processing question in an interview for 20-30 minutes, one very excellent answer should be, "sorry man, not gonna try to do this in 20 minutes because in reality I know there are string handling landmines I would need to research and slowly process, and I would never believe this is worth committing to memory for a short interview."
• MrBingley 8y ago
  Looking at the comments in this post, I'm resigning myself that there simply is no correct solution for copying or concating strings in C. Null-terminated strings are a fundamentally broken concept. I think the long-term solution is simply to move to a different language (Rust, C++, D, Go, whatever) where we have the benefit of hindsight and have (pointer, length) string types, which solve all the problems null-terminated strings introduce.
  • mjevans 8y ago
    You could also validate on interface transition and use your own type internally; which is often what happens when using another language and exporting C style library bindings.
  • blub 8y ago
    It's quite sad to behold how hard it is to work with strings in C. All those functions seem broken by design, forcing the programmer to clean up their mess in edge cases.
• deleted 8y ago
  [deleted]
• jacquesm 8y ago
  Bad advice is worse than no advice at all.
• loeg 8y ago
  Use strlcpy/strlcat instead.[0] strlcpy takes the full size of the destination buffer, limits the copy to N-1, and nul-terminates the result for you. It's like the "correct" example in TFA, but with less annoying boilerplate. Some more verbose design/rationale for the really curious.[1] Another thing to keep in mind is the sometimes surprising behavior of strncpy(large_buffer, short_string, sizeof(large_buffer))[2]: If the length of src is less than n, strncpy() writes additional null bytes to dest to ensure that a total of n bytes are written. Strlcpy doesn't do that. Just use strlcpy. On Linux, it can be found in the libbsd package. [0]: https://www.freebsd.org/cgi/man.cgi?query=strlcpy&sektion=3 https://www.freebsd.org/cgi/man.cgi?query=strlcpy&sektion=3 [1]: https://www.sudo.ws/todd/papers/strlcpy.html https://www.sudo.ws/todd/papers/strlcpy.html [2]: https://linux.die.net/man/3/strncpy https://linux.die.net/man/3/strncpy
• rini17 8y ago
  1. Avoid sizeof, lest someone comes and changes the array into a pointer. Use constant parameter instead: #define BUFLEN 8 char buffer[BUFLEN]; strncpy(...etc... BUFLEN-1); 2. Check length of the string to be copied BEFORE copying and if too long, fail (using assert, exit or so) instead of silent truncation or worse. Why it is so hard and instead new "safe" string functions must be invented?
  • kazinator 8y ago
    There is no absolutely foolproof way to code in C such that no matter how someone changes the program, they will be spared from making a mistake. If we just program for today, sizeof buffer is much better than proliferating a preprocessor constant that may or may not correctly reflect the object being overwritten. For the silly mistake of changing an array to a pointer without taking care of sizeofs, GCC gives us some diagnostics: -Wsizeof-pointer-memaccess Warn for suspicious length parameters to certain string and memory built-in functions if the argument uses "sizeof". This warning warns e.g. about "memset (ptr, 0, sizeof (ptr));" if "ptr" is not an array, but a pointer, and suggests a possible fix, or about "memcpy (&foo, ptr, sizeof (&foo));". This warning is enabled by -Wall. -Wsizeof-array-argument Warn when the "sizeof" operator is applied to a parameter that is declared as an array in a function definition. This warning is enabled by default for C and C++ programs.
• bitmadness 8y ago
  Simple solution to all these problems: stop using shitty languages like C.
• lkjalksdjfasdf 8y ago
  I don't use pure C for string handling anymore. I use C++ and extern C ABI. Rust can also work. C++ can infer the destination size via templates.
  • pjmlp 8y ago
    Some of us were already doing it in 1992, with our own string classes as passage rite, but adoption takes time.
• rurban 8y ago
  Interestingly even the Annex K strncpy_s and strncat_s are unsafe by design, that's why I only added them to the safeclib via --enable-unsafe. But recently I got fed with all this unsafety nonsense with the truncating variants and changed the implementation to always terminate the asciiz string properly. https://github.com/rurban/safeclib/blob/master/src/str/strncat_s.c https://github.com/rurban/safeclib/blob/master/src/str/strnc...