9 ms·
Stop using strncpy already (2013)
- dang 8y agoDiscussed at the time: https://news.ycombinator.com/item?id=5491121 https://news.ycombinator.com/item?id=5491121
- gp7 8y agoTwo years after this post, the Linux kernel added strscpy, the api of which is equivalent to the safe strncpy in this post. Internally, it stops copying once it reaches the null terminator. https://lwn.net/Articles/659214/ https://lwn.net/Articles/659214/
- sevensor 8y agoThe article's proposed implementation uses a template to guarantee that egregious mistakes will fail to compile. This is clever, but it's C++. The kernel's version isn't quite as safe because it can overrun the destination. In principle it's impossible for it to be safe, because this is C, and neither the compiler nor the programmer can tell how big the destination is. We have to rely, as the article complains, on the programmer calling the function to know how big the buffer is.
- IChooseYou 8y agoAlso Microsoft's strncpy_s has been around forever.
- brucedawson 8y agoIt's too bad strncpy_s is so unwieldy to use. Having to pass the TRUNCATE flag is frustrating. I really want a two argument (dst, src) function that guarantees null termination.
- rurban 8y agoBut Microsoft's strncpy_s is unsafe. My safeclib also had the unsafe versions behind the non-default ./configure --enable-unsafe switch. But this year I decided to make them safe instead. In this case the spec is broken.
- iamthad 8y agostrscpy seems like a nice interface. Is an implementation outside of the kernel available? Preferably one that is permissively-licensed? Also, it doesn't appear to be specified what happens if the count argument is too large to be represented as a ssize_t. The destination buffer would have to be extremely large, so it probably doesn't happen in practice, but it'd be good to specify it, or at least explicitly state it's unspecified / undefined. https://www.kernel.org/doc/htmldocs/kernel-api/API-strscpy.html https://www.kernel.org/doc/htmldocs/kernel-api/API-strscpy.h...
- ndesaulniers 8y agoI've had patches rejected from the Linux kernel that tried to switch away from strncpy in favor of strlcpy. The behavior that strncpy continues to zero out the rest of the destination in the case where size of source is less than destination was being relied upon to not leak uninitialized memory to userspace. Seems strscpy alone suffers the same.
- beefhash 8y ago> strlcpy is designed to solve the null-termination problems – it always null-terminates. It’s certainly an improvement over strncpy, however it isn’t natively available in VC++. Let's not forget that it's also not natively available in glibc. Though you'll find it in every BSD you can think of.
- jandrese 8y agoSadly, it still has this braindamage: Like snprintf(3), the strlcpy() and strlcat() functions return the total length of the string they tried to create. For strlcpy() that means the length of src. For strlcat() that means the initial length of dst plus the length of src. So if you've mmaped in something like the OpenStreetMaps xml dump file and try to strlcpy out the first 100 bytes (because you didn't want to have to null terminate it yourself after a memcpy), you'll notice that your program is running rather slowly.
- kevindqc 8y ago"Let's read all the data you don't want because!" nice
- needlepont 8y agoThat is trusting user input. You don't do that, right?
- quietbritishjim 8y agoIt's not about trusting user input. It's asking for a function to create a 100 byte substring of a very long string and expecting it to take time~100 not time~len(src). In fairness, that is probably just not the target "market" for strlcpy. Presumably, it is meant for "please copy this whole string which I expect to fit in the target (but catch me in the rare case that it does not)".
- needlepont 8y agoAs you note: your example is specious.
- umvi 8y agoI feel like there are a lot of good alternatives to C. What we did on our team was get our C compiling under g++, and then refactor as C++14. Now string copies are trivial and safe. I'm sure it would have been even safer to switch to Rust or something, but no one in my company has any experience with that, while there were a few C++ gurus.
- jstimpfle 8y agoWhat are the good alternatives? You can either copy memory manually, or have a system like C++'s that causes lots of tiny allocations, which is very inefficient. Both in space and time. You can also opt for immutable strings to be able to share strings more conveniently and efficiently, at the cost of O(n^2) build time (where automatic optimizations fail) or decreased convenience (explicit string-stream aka string builder). If efficiency is not a concern for you and you rather like the convenience, good for you. Personally I feel plain const char pointers (plus length, depending on expected size) are good enough to emulate the immutable string type. For modifiable strings, I still prefer viewing them as chunks of memory, using char pointer + length + capacity. Nice thing is, you can easily go from the former to the latter in the lifetime of any one string. You can also pool many strings in large chunks if you build only one string at any one time.
- adrianN 8y agoWhatever alternative reliably prevents buffer overflows, so pretty much anything but char*.
- pjmlp 8y agoYou care about that after a profiler proves that it is actually a problem to the expected delivery SLAs for the application. Micro-optimizing each line of C code based on gut feeling is not an option if quality matters.
- jstimpfle 8y agoIt's not about the code, but about choice of data structure. Unfortunately that has to be planned ahead to some degree. You can't really optimize it after the fact. Typically a choice affects more than a single line of code, but that doesn't mean "microoptimizing every line of code", which is obviously a bad idea.
- AdmiralAsshat 8y agoIn principle, I feel like "Ignore the language-provided standard libraries and use this little macro I wrote in all your code" is bad advice. If the standard libs are that bad, the community should really lobby for the inclusion of a "safe" string copy into the C standard. Failing that, use a battle-tested third-party library like bstrlib[0]. [0]http://bstring.sourceforge.net/ http://bstring.sourceforge.net/
- jstimpfle 8y agoStrings are so tiny an abstraction over plain memory that every C programmer ends up doing them their own way. And that can be a good thing. I already proposed a couple of ways to represent or muck with strings in my other comment.
- cliffy 8y agoHow is that a good thing? There are ways of abstracting/handling strings that are strictly safer than other ways. In an unsafe language like C, you're inviting problems by trying to do things in your own idiosyncratic way.
- jstimpfle 8y agoSometimes performance or flexibility are important concerns. But feel free to manage millions of small strings with std::string or even as GC'ed objects :). And incur the overhead of indirection when attaching bounded-size strings to other data.
- ahoka 8y agostrncp is fine, however it was never meant for null-terminated strings! A safe alternative is using snprintf and checking the return value for truncation.
- kevindqc 8y agohttps://lwn.net/Articles/225108/ https://lwn.net/Articles/225108/
- brucedawson 8y agosnprintf is hardly safe. It does guarantee null termination, so that is progress, but it requires the caller to pass the buffer size. As I mention in the "More typing means more errors" section of the article software developers are endlessly creative about ways to pass the wrong buffer size. They usually pass the right buffer size, but 99.9% correct still means a lot of bugs. These bugs can be avoided by using a template function to infer the destination buffer size. A template wrapper around snprintf would work very nicely.
- bigpicture 8y ago> These bugs can be avoided by using a template function to infer the destination buffer size I'm a big fan of using strategies to avoid bugs. Another strategy than can be a big help is to spend a little time thinking about what kinds of strings you are going to be using and how big they might be. And then sanitizing your program input based on those sizes. Find the too-long strings before they get into your program and before you start copying them around. The system I'm responsible for came into being as a set of programs on a Univac II and was a bunch of C and FORTRAN when I joined. Because of the need to pass data between C and FORTRAN programs/code, we had a set of 10-20 standard string sizes, based on what kind of text you had. We no longer write new code in either C or FORTRAN, but maintain the concept of standard sizes for our text. By specifying the kind of text (we also do this with numbers) in the design steps, we save far more time and effort than we spend over the entire SDLC.
- hzhou321 8y agoIf we preset the end of buffer with '\0' and then assume the buffer having 1-less capacity, wouldn't it address the strncpy issue?
- needlepont 8y agoOf course, but everyone wants to overdo the complexity of the time worn solution. OMG you need to null terminate the string after all the _other_ gymnastics..gee C sure does suck! Why don't we use rust|go|c++ ad-nauseam. bzero(buf,sz); /memset nazis here/ strncpy(buf,src,sz - 1);
- bitwize 8y agoIt doesn't make sense to have to null-terminate the string by hand (maybe) after doing a "safe" STRING copy. Which means it's easy to forget to do and that's a dangerous wart in the design. Really, stop using C. To quote Hayao Miyazaki, C was a mistake.
- needlepont 8y agoTeach your grandmother to suck eggs and resign the thought police please.
- dang 8y agoPosting like this will get you banned here. Please review and follow the rules: https://news.ycombinator.com/newsguidelines.html https://news.ycombinator.com/newsguidelines.html.
- hzhou321 8y agoWhat I meant is more like: #define BUFSIZE 100 char buf[BUFSIZE+1] buf[BUFSIZE]=0 // for global var, it is already zeroed, right? Then one can call strncpy(buf, BUFSIZE, ...) without worrying about null-termination.
- sevensor 8y ago
- hedora 8y agoI don’t understand why silently truncating strings is considered to be acceptable practice. If you have the length of both, simply assert the length of the destination is <= the length of the source. Problem solved (bugs become obvious in testing).
- brucedawson 8y agoWhat about user input, strings coming from files, and in general the diversity of things that can go wrong when your product ships. This is particularly important if your product is attacked by those who would like to exploit it. Asserts are necessary, but not sufficient.
- russdill 8y agoBecause the API is stupid and not intuitive. It's for memory buffers where the string is either NULL terminated or it is the length of the memory buffer. This saves an extra byte. yay?
- russdill 8y agoI'll add that it's really handy for that specific situation as it also avoids leaking uninitialized bytes.
- gweinberg 8y agoThe more I think about this, the more sense it makes to say you should check the length of the buffer before they copy. I'm not sure an assert is always right, exactly what should be done if the string doesn't fit may depend on what the program is doing. But silently truncating is generally not the right thing to do.
- brucedawson 8y agostrcpy_s does this. It has a template variant to infer the length of the destination buffer and it will terminate the program if the source string is too large. If the destination buffer doesn't have a fixed length then it will fail to compile. This is similar to your suggestion with these improvements: - Template deduction of the destination size is done - this is essential to avoid bugs - Detection of overwrites is done in release builds as well as during testing - Detection of overwrites is automatic - it doesn't require manually adding asserts to call sites - strcpy_s exists in some standard libraries
- mcguire 8y agoBeen a while since I used C++... How does this work if the size of the destination isn't known at compile time? I.e., it's not "buffer[5]”?
- yarosv 8y agoThe author states > The focus of this post is on fixed-size string buffers, but the technique applies to any type of fixed-length buffer.
- brucedawson 8y agoYou get a compile error. And this is progress. I have fixed code that looked something like this: strncpy(dst, sizeof(dst), src); data[sizeof(dst)-1] = 0; See the bug? Of course not, because it's not visible, but I've seen this pattern used where dst is a pointer. This means that four or eight bytes (three or seven if you don't count the null terminator) gets copied, regardless of how many bytes dst points to. With strcpy_safe an attempt to copy to dst would not work and the developer would have to fix their code. Every mistake that can be made, will be made.
- wmil 8y agoI feel like all of these articles can be summed up with: "C strings and string functions aren't designed to be safe. If you want safe strings use a library."
- deleted 8y ago[deleted]
- kodis 8y agoI'd say "Stop using C and C++ already" or "Stop using null terminated strings already", but I recognize that those are not always practical solutions. I do have some maintainability concerns though about the promulgation of numerous competing home-brew solutions to a near trivial problem throughout a code base.