4 ms·
I once got called in to fix an SS7 stack suffering from poor performance. Pretty well written, and not obvious at first sight why it was going slow. Most of it
by zh3 4y ago
I once got called in to fix an SS7 stack suffering from poor performance. Pretty well written, and not obvious at first sight why it was going slow. Most of it was low-level bit fiddling, and some small strncpy's() - generally about 8 chars or so.
Didn't take that long to profile (well, printf's as no profiling available) and figure out it was the strncpy's causing the problem, but why? Well, there was a handy 8 megabyte buffer used for working memory that the strings were being copied into that for modification.
From the strncpy() man page:-
>If the length of src is less than n, strncpy() pads the remainder of dest with null bytes.
Ah. So every little strncpy was essentially copying the string then zeroing out 7,999,992 bytes. And there were lots of little strncpy's...
- PaulDavisThe1st 4y agoThe appearance of strncpy() in any source code is an immediate panic attack for me. It should never be used, and if it is used, it should be removed. Similar rule for sprintf(), all instances of which should be replaced by snprintf().
- bluGill 4y agoUnfortunately there often isn't a better replacement in your standard library (embedded systems are weird). I ended up using strncpy followed by automatically setting the last byte of the string to null.
- PaulDavisThe1st 4y agostrncpy() in particular is so bad that you're better off (for a rare exception to the rule) just writing your own, that does what most people think strncpy() does (or should do) rather than what it actually does.
- saagarjha 4y agoWrap memccpy.
- saagarjha 4y agosnprintf has very similar performance pitfalls.
- PaulDavisThe1st 4y agono, because the size argument is only an upper bound on how many bytes can be written into the destination. snprintf (huge_buf, huge_buffer_size, "%d", 1); will write two bytes into huge_buf, regardless of huge_buffer_size (assuming it is 2 or larger).
- saagarjha 4y agoIt’s in the other direction, snprintf(small_buf, small_size, “%s”, huge_string) will need to iterate the whole string.
- PaulDavisThe1st 4y agowhy? snprintf() will just write as many bytes from huge_string as necessary, up to the smaller of small_size and strlen (huge_string). what makes you believe it will iterate the whole of huge_string?
- saagarjha 4y agoQuoth the standard: > The snprintf function returns the number of characters that would have been written had n been sufficiently large, not counting the terminating null character, or a negative value if an encoding error occurred. In essence it needs to return strlen of huge_string even though very little of it was actually written.
- PaulDavisThe1st 4y agoWell that's pretty fucked up. I note that the GNU C library docs say this: > Attention: In versions of the GNU C Library prior to 2.1 the return value is the number of characters stored, not including the terminating null; unless there was not enough space in s to store the result in which case -1 is returned. This was changed in order to comply with the ISO C99 standard. ISO C99 needs a kick in the head. Yes, there is a use case for this return value (buffer wasn't large enough, reallocate and try it again). But wow, own goal team! Thanks for this. I had no idea that C99 had defined this so stupidly. I do see that in 2004, the linux kernel added replacements (scnprintf() which behave as the pre-C99 versions of snprintf generally did). There's a good discussion of this here: https://lwn.net/Articles/69419/ https://lwn.net/Articles/69419/