Re:Unix v6 Ported to ANSI C
by ssp on Tuesday November 08, 2011 @04:23PM
(#3212873)
There is a bug in the memmove() function from ulib.c:
If the two blocks overlap, and the address of dst is greater than the address of src, then parts of the source will be overwritten before it can be copied.
[ Parent
| on HN ]
Re:Unix v6 Ported to ANSI C
by robinhouston on Tuesday November 08, 2011 @04:33PM
(#3212921)
The original v6 doesn’t have any of the mem* functions, but its bcopy implementation has the same issue. Presumably the convention that bcopy permits overlap had not been established at that time.
I'm not massively great at C but would this be a fix?
void*
memmove(void *vdst, void *vsrc, int n)
{
char *dst, *src;
dst = vdst;
src = vsrc;
if(*vdst > *vsrc) {
*vdst += n;
*vsrc += n;
while(n-- > 0)
*dst-- = *src--;
}
else if(*vdst < *vsrc)
while(n-- > 0)
*dst++ == *src++;
return vdst;
}Ok, updated version:
void*
memmove(void *vdst, void *vsrc, int n)
{
char *dst, *src;
dst = vdst;
src = vsrc;
if(vdst > vsrc) {
dst = vdst + n;
src = vsrc + n;
while(n-- > 0)
*--dst = *--src;
}
else if(vdst < vsrc)
while(n-- > 0)
*dst++ == *src++;
return vdst;
}
Does this solve the bugs?
Re:Unix v6 Ported to ANSI C
by abecedarius on Wednesday November 09, 2011 @11:09AM
(#3216022)
There's an == for an =. Also vdst+n is an error, I believe, because sizeof *vdst has no meaning.
Re:Unix v6 Ported to ANSI C
by abecedarius on Tuesday November 08, 2011 @08:00PM
(#3213722)
At least two bugs: the test should compare vdst to vsrc, not * vdst to * vsrc, and in the first branch you're copying vsrc[n] which is out of range (and skipping vsrc[0]).
The first bug I'm confused? Surely we want to use the * because we're comparing the position in memory, not what's in the memory? Or does the * mean "compare what is in that memory slot"?
Second bug, cheers.
Re:Unix v6 Ported to ANSI C
by simcop2387 on Tuesday November 08, 2011 @08:52PM
(#3213876)
I see an additional bug in there compared to the original, you're copying the memory below vsrc into the memory below vdst in the first branch. you need to add to dst and src not vdst/vsrc.
Thanks, C rather rusty :)
reminds me of the intel comment in their implementation of memcpy, something like 'no overlay check as implicit convention'
Re:Unix v6 Ported to ANSI C
by microtherion on Tuesday November 08, 2011 @05:34PM
(#3213183)
Yes, the standards say that the behavior of memcpy, as opposed to memmove, is undefined when the arguments overlap.
Right. Quoting from the manpage:
> The memory areas may overlap: copying takes place as though the bytes in src are first copied into a temporary array that does not overlap src or dest, and the bytes are then copied from the temporary array to dest.
http://www.kernel.org/doc/man-pages/online/pages/man3/memmov...
Notice how it says 'as though'; I can't see why doing the copy backwards (from end to start) wouldn't work, myself.
If it's higher, move from the front. If it's lower, start at the back. You have to check each time you do it.
Still doesn't work going end to start.
src starts at 50, ends at 100
dst starts at 25, ends at 75.
First we copy 100 to 75, then 99 to 74, etc ... now we get to copy from 75 to 50, but we've overwritten it already!Linux isn't Unix, so are you sure it's the same?
memmove() is in ANSI C standard.
UNIX v6 predates ANSI C by over a decade I think.
Here is a standard implementation I found from some code at work.
/* memmove.c -- copy memory.
Copy LENGTH bytes from SOURCE to DEST. Does not null-terminate.
In the public domain.
By David MacKenzie <[email protected]>. */
#if HAVE_CONFIG_H
# include <config.h>
#endif
void *
memmove (dest, source, length)
char *dest;
const char *source;
unsigned length;
{
char *d0 = dest;
if (source < dest)
/* Moving from low mem to hi mem; start at end. */
for (source += length, dest += length; length; --length)
*--dest = *--source;
else if (source != dest)
{
/* Moving from hi mem to low mem; start at beginning. */
for (; length; --length)
*dest++ = *source++;
}
return (void *) d0;
}