strncpy implementation too complicated in glibc

Question

I'm trying to understand string.h functions. Here is my own implementation of strncpy()

char * my_strncpy(char *dst, const char* src, int n)
{
    char *orig = dst;
    const char *hold = src;
    int count = 0, remain = 0;
    while(*(hold++))
            count++;
    if ( n > count )
    {
            remain = n - count;
            n = count;
    }
    while(n--)
            *dst++ = *src++;
    while(remain--)
            *dst++ = '\0';
    return orig;
}

But while looking at glibc implementation here, I'm wondering why it is too big and complicated.

I tested for execution time using "time" command. Both functions run almost same.
Can someone share knowledge on glibc strncpy() and what I'm missing in my_strncpy().

It is a little more complicated because, AFAICT, it does not count the chars before it copies, and it seems to have some kind of simple loop unrolling going on. Not sure if that is really much faster, though. You'd have to time/profile both to find out. — Rudy Velthuis, Jun 18 '14 at 06:36
Aparently it copies 4 `char`s on every for loop iteration. After finishing copying these 4-char chuncks, the code at `last_char` is used to copy the remanining (<4) characters. Ultimately the destination buffer is filled with `\0`s. The logic does not seem very complicated to me, however I'm not sure of the potential performance benefits. — dragosht, Jun 18 '14 at 06:41
The glibc version will have a distinct speed advantage the longer the string and the larger the number of characters to be copied. Generally the additional code gymnastics are to prevent branching at the assembly level which is what provides the speed advantage. For another good example, look at the implementation of `strlen` — David C. Rankin, Jun 18 '14 at 06:42
I suppose that you have forgotten something in the while(remain--) loop. — mch, Jun 18 '14 at 06:42
@DavidC.Rankin: I assume you are right. It should not be faster for short strings, but it might outperform the rather simple implementation posted here for long strings. — Rudy Velthuis, Jun 18 '14 at 06:50
I'm not an expert on their reasoning by any stretch, but reading a number of their implementations, there is usually sound reasoning why they do what they do (aligning on boundaries, etc..). Other times, the glibc functions are just hold-overs from days-gone-by as well. — David C. Rankin, Jun 18 '14 at 06:54
I calculated time for both, result was same. I didn't try for longer string of-course. Thanks all, i got some proper reason. — Siva Padhy, Jun 18 '14 at 07:10
@Manül: in while(remain--) loop i am just filling zeros in the remaining spaces after copy, as the standard strncpy() implementation says. — Siva Padhy, Jun 18 '14 at 07:18
@DavidC.Rankin: You are right. I calculated execution time for bigger string. Here is the difference. glibc implementation: real 0m0.018s user 0m0.016s sys 0m0.000s My Implementation real 0m0.119s user 0m0.116s sys 0m0.000s — Siva Padhy, Jun 18 '14 at 07:23
@SivaPadhy, are you sure? you don't fill the remaining spaces, you are always NULL-terminating in the same position in your `while(remain--)` — David Ranieri, Jun 18 '14 at 07:24
@AlterMann: Thanks for the observation.I just missed to increment the pointer. Modified. — Siva Padhy, Jun 18 '14 at 07:29
Generally when timing functions against glibc implementations, try at least 1000000 iterations and test against short strings (<16 chars) and longer strings (>32 chars), etc.. Your implementations for short strings may even be faster, but I've not been able to beat glibs implementations for long strings. — David C. Rankin, Jun 18 '14 at 07:29
Related: [How the glibc strlen() implementation works](http://stackoverflow.com/questions/20021066/how-the-glibc-strlen-implementation-works), — Yu Hao, Jun 18 '14 at 07:34
Your function invokes undefined behavior if passed the address of a non-zero-terminated array of characters, which is allowed by the standard, and definitely corresponds to actual practice. This also mean it is possible to make your version very slow with long source strings. — Pascal Cuoq, Jun 18 '14 at 08:54

score 1 · Answer 1 · edited May 23 '17 at 10:24

1

From a modern C programming perspective, that "Glibc" code is very badly written. It looks like a collection of premature optimizations for one particular platform with 32 bit alignment. If the compiler is crap, then you'll have to group bytes in units of the preferred alignment and copy them one such unit at a time. That's the main reason why the code looks so weird.

I would guess that the code was likely written a long time ago, when compilers were a lot worse at optimizing code and CPUs had less hardware support for things like these. The inconsistent, seemingly random way that they switch back and forth between prefix to postfix increment also suggests that the code was written for a poor compiler.

Apart from pre-mature optimization, the code is complete spaghetti, which there is no sensible explanation for. The complete lack of comments also suggests that the code was written by a bad programmer.

So to sum it up, there may or may not be various historical reasons why they wrote the code in this way, but there are lots of bad programming practice in the code which can't be dismissed as pre-mature optimizations.

Just dismiss that code as rubbish.

Also please note that the strncpy function is mainly obsolete and should be avoided. strncpy was only meant to be used for an ancient string format in Unix. It was never intended to be a safe version of strcpy. On the contrary, the function is dangerous and known to cause a lots of bugs because of accidental missing null terminations.

The standard specification of strncpy also forces it to do a lot of pointless things, like checking for null termination when you already know the length in advance. Also, one may wonder what good it does anyone to fill the remaining characters after the first \0 with even more \0. All of these pointless requirements make the function needlessly slow.

So there is never a reason to use strncpy anywhere in modern C code. The proper way to copy strings of a known length is this:

memcpy(str1, str2, str2len + 1);

edited May 23 '17 at 10:24

Community

1
1

answered Jun 18 '14 at 07:34

Lundin

195,001
40
254
396

`strncpy()` is still good for explicitly null-padding a string to a certain number of bytes. – The Paramagnetic Croissant Jun 18 '14 at 07:42
@user3477950 No it isn't. Use memcpy() followed by memset(). – Lundin Jun 18 '14 at 07:42
2

why isn't it? Also, I'm not sure about your note about premature optimization either. This is library code, there definitely is some excuse for writing more obscure but faster code so that *users* of the library don't have to hand-craft their version of optimized functions because the library functions are insufficiently fast. The lack of comments may be a point, but this implementation of `str(n)cpy` is quite common and anyone who has encountered the switch-trick will recognize it, so there's no much need for comments at all. – The Paramagnetic Croissant Jun 18 '14 at 07:46
@user3477950 Because strncpy is unsafe and slow. I've already given the reasons in my answer. And no, of course there is no excuse for programmers of libraries to write obfuscated code with no comments or other documentation. Generally, it does not make sense to optimize code on the high level of libraries, if you want the code to be portable. Unless of course, this code is specific for a certain compiler port. In that case, it most likely makes more sense to write it in assemble, which in my experience is far more common practice for the standard library copy functions. – Lundin Jun 18 '14 at 07:55
@Lundin, `memcpy` seems as dangerous as `strncpy` (should not overlap ), do you mean `memmove`? – David Ranieri Jun 18 '14 at 07:56
3

@Lundin `strncpy()` is unsafe if you don't use it properly. Just like almost any other C function dealing with strings. As to its speed: if it's "slow", then so is `memcpy()` and `strcpy()`. That's not a reason. "Generally, it does not make sense to optimize code on the high level of libraries" - it absolutely positively does. Optimization should happen in libraries so that user code doesn't have to worry about that. The trick glibc implements works (and is often used) on most 32-bit systems (Linux, BSD, OS X and even Windows). – The Paramagnetic Croissant Jun 18 '14 at 08:03
@AlterMann The overlap issue is there for most functions in the standard. Most often overlapping is not a problem. In the very rare case where it is, you can indeed use memmove. The main concern about `strncpy` being unsafe is [the null termination issue](http://stackoverflow.com/questions/869883/why-is-strncpy-insecure). – Lundin Jun 18 '14 at 08:04
@user3477950 Again, the optimization should happen in _the compiler port of the library for one specific platform_. Also 64 bit CPUs have been on the market for quite a while, making this code pre-mature optimization for historical reasons. – Lundin Jun 18 '14 at 08:07
+1 `strncpy` is susceptible to a variety of exploits, @downvoter, care to explain? – David Ranieri Jun 18 '14 at 08:15
As usual on Stack Overflow, the down vote likely originates from above the comment debate, rather than because of something in the actual answer. – Lundin Jun 18 '14 at 08:25
3

Full story of `strncpy`: [How can code that tries to prevent a buffer overflow end up causing one?](http://blogs.msdn.com/b/oldnewthing/archive/2005/01/07/348437.aspx). – Maxim Egorushkin Jun 18 '14 at 08:32
Wait, what? `memcpy(str1, str2, str2len + 1);` is just a long way to write `strcpy(str1, str2);` – domen Aug 11 '18 at 07:21
@domen No, strcpy checks for null termination, so it is considerably slower with the extra checks and branches. – Lundin Aug 13 '18 at 06:23

strncpy implementation too complicated in glibc

1 Answers1