generic strncpy - off-by-one error

You're all wrong. This is some kind of programming test for sure!

Let us imagine that glibc has a correct version. By exhaustive testing, I found a version that works. Here it is, along with the test code:

////////////////////////////////////////////////////// #define _GNU_SOURCE
#include <string.h>
#include <stdlib.h>
#include <stdio.h>

// first correct implementation!
char * strncpy_good(char *dest, const char *src, size_t count){
char *tmp = dest;
memset(dest,'\0',count);
while (count-- && (*tmp++ = *src++))

    ;

return dest;
}

static char ref[32];
static char hmm[32];

static char s0[] = "";
static char s1[] = "1";
static char s2[] = "12";
static char s6[] = "123456";
static char s7[] = "1234567";
static char s8[] = "12345678";
static char s9[] = "123456789";

static void tester(const char *src, size_t count){
memset(ref,'%',sizeof ref); memset(hmm,'%',sizeof hmm);
strncpy (ref+2,src,count);
strncpy_good(hmm+2,src,count);

  if(memcmp(ref,hmm,sizeof hmm)){
    printf("error @ size %d\n",(int)count);

}
}

int main(int argc, char *argv[]){
int i = 15;

  while(i--){
    tester(s0,i);
    tester(s1,i);
    tester(s2,i);
    tester(s6,i);
    tester(s7,i);
    tester(s8,i);
    tester(s9,i);

}
return 0;
} ////////////////////////////////////////////////////////////

char *strncpy(char * s1, const char * s2, size_t n)

{
    register char *s = s1;
    while (n) {
	if ((*s = *s2) != 0) s2++;
	++s;
	--n;
    }
    return s1;
}

-Erik

--
Erik B. Andersen http://codepoet-consulting.com/
--This message was written using 73% post-consumer electrons--

That's excellent. On ppc I count 12 instructions, 4 of which would go away for typical usage if inlined. Annoyingly, gcc doesn't get the same assembly from my attempt at that general idea:

char * strncpy_5(char *dest, const char *src, size_t count){
char *tmp = dest;

  while (count--){
    if(( *tmp++ = *src )) src++;

}
return dest;
}

I suppose that gcc could use a bug report.

Its has different semantics though. Well taken as a whole they are the same. When your loop finishes, count will be -1.

I often noticed that using '++' and '--' within or just before assignments and/or comparisons often break the code and make it suboptimal. C provides enough flexibility to code what you think nearly at the instruction level. Since 'while' loops often start with a jump to the end, you can sometimes help the compiler by enclosing them within an 'if' statement such as below. BTW, in your case, count ends with -1.

I've absolutely not tried this one, but it could produce different code on your PPC, and can trivially be derived to cleaner constructs. I proceeded the same way when I wrote my own optimized strlcpy() implementation which is 45 bytes long and copies 1 char per CPU cycle on i686.

char *strncpy(char *dest, const char *src, size_t count) {

   if (count) {
      char *tmp = dest;
      while (1) {
         *tmp = *src;
         if (*src) src++;
         tmp++;
         if (!count--) break;
      }

}
return dest;
}

Cheers,
Willy

Nice!

How about this:

char *strncpy(char * s1, const char * s2, size_t n)

{
	register char *s = s1;

	while (n && *s2) {
		n--;
		*s++ = *s2++;
	}
	while (n--) {
		*s++ = 0;
	}
	return s1;
}

This reminds me a lot of the ORIGINAL, although I didn't pay much attention to it at the time, so I don't remember. It may be that the original had "n--" in the while () condition of the first loop, rather than inside the loop.

I THINK the original complaint was that n would be off by 1 upon exiting the first loop. The fix is to only decrement n when n is nonzero.

If s2 is short enough, then we'll exit the first loop on the nul byte and fill in the rest in the second loop. Since n is only decremented with we actually write to s, we will only ever write n bytes. No off-by-one.

If s2 is too long, the first loop will exit on n being zero, and since it doesn't get decremented in that case, it'll be zero upon entering the second loop, thus bypassing it properly.

Erik's code is actually quite elegant, and its efficiency is probably essentially the same as my first loop. But my second loop would probably be faster at doing the zero fill.

Now, consider this for the second loop!

	while (n&3) {
		*s++ = 0;
		n--;
	}
	l = n>>2;
	while (l--) {
		*((int *)s)++ = 0;
	}
	n &= 3;
	while (n--) {
		*s++ = 0;
	}

This is only a win for relatively long nul padding. How often is the padding long enough?