strncpy doesn't pad the destination string with NULs

[hamish]

See the relevant standard: ​http://pubs.opengroup.org/onlinepubs/9699919799/functions/strncpy.html

[hamish]

On x86, gcc2+4 O0/O2/O3, zeroing everything beforehand is faster than zeroing the remainder afterwards in most cases

[korli]

Replying to hamish:

On x86, gcc2+4 O0/O2/O3, zeroing everything beforehand is faster than zeroing the remainder afterwards in most cases

Hmm, surely correct when the source string is empty, but incorrect when the source string has the specified length or more.

I think you should better memset afterward with dest and count, which should already have the correct values (unless I'm mistaken).

[hamish]

dest and count won't have the right values afterwards. The loop semantics need to be changed a little so that count doesn't wrap around. Something like:

while (count && (*dest = *src++) != '\0') {
  dest++; count--;
}

So memsetting before with the original loop turns out a little faster in most cases (other than where the buffer gets almost entirely filled). The difference is small though, so I'm OK to memset after.

[korli]

What about this?

while (count--) {
	if ((*dest++ = *src++) == '\0') {
		memset(dest, '\0', count);
		break;
	}
}

[tqh]

You could take a peek at strlen or strnlen which checks four bytes at a time for null, so you don't have to do it bytewise.

[hamish]

Replying to korli:

What about this?

Performs about the same as the loop in comment 4.

Checking four bytes at a time beats the others by a long shot at -O0, but only seems a little faster, if at all, at -O3.

[tqh]

Interesting probably hard to do better, maybe align writes on four bytes to ease up on the write combining.

Btw I've been looking at -D__NO_STRING_INLINES as a build parameter, which from what I can tell, tells gcc to not trust glib with inlining but do it in gcc. This might be the wrong assumption though as it is very poorly documented.

[hamish]

Replying to tqh:

Interesting probably hard to do better, maybe align writes on four bytes to ease up on the write combining.

Yep, I'm already aligning the destination buffer.

Btw I've been looking at -D__NO_STRING_INLINES as a build parameter, which from what I can tell, tells gcc to not trust glib with inlining but do it in gcc. This might be the wrong assumption though as it is very poorly documented.

As far as I can tell that only affects whether glibc's string.h defines a bunch of inline assembly versions of the string functions. Since we don't use glibc's string.h I don't think it has any effect.

[tqh]

The code can be simplified by changing

34	    if (((addr_t)src & 3) != 0) { 

to

34	    if (((addr_t)src & 3) == 0) { 

removing line 35 to 45 and moving

        // Deal with the remainder. 
 	60	        while (count-- != 0) { 
 	61	            if ((*dest++ = *src++) == '\0') { 
 	62	                memset(dest, '\0', count); 
 	63	                return tmp; 
 	64	            } 
 	65	        } 

Also I'm not sure whether we want to do source alignment (on x86 at least).

[hamish]

To my understanding, unaligned access is not necessarily OK on all architectures. Given that this is a generic strncpy it's probably a good idea to avoid it?

[tqh]

Yes, mine too. I was fishing for comments on if we should have special case for x86. Probably not worth it.

[tqh]

Looks good to me.

[tqh]

Btw we need source aligned so that the source word never steps over a page boundry if it reads a byte or three past the \0.

[pdziepak]

Replying to tqh:

Yes, mine too. I was fishing for comments on if we should have special case for x86. Probably not worth it.

Functions like strncpy, strncmp, strcpy, memcpy, memcmp, etc are usually implemented in assembly (thus there are different versions of them for each architecture). Apparently it is still worth to do that. I wrote a ​simple test and compiled it on gcc 4.6.3. memcpy1 takes about 300ms to complete, while memcpy2 needs only 100ms. Obviously, memcpy is the easiest of these functions to make run much faster when implemented in assembly, but then again it may be worth to consider such option.

[tqh]

Yes, I've been playing a bit with that as well in my spare time. Patch applied in hrev44069, nice work.

[tqh]

Unfortunatly it's not presented that well with the rename I did from .c to .cpp. Should have pushed in two steps. Sorry.