memcpy

[Andy Lester]

Is there a reason we use

memcpy( dest, src, sizeof(FOO) );

instead of

*dest = *src;

The latter should be the exact same code, but be much less likely to
be screwed up.

At the very least we should be using:

memcpy( dest, src, sizeof(*dest) );

Of course, I'm only talking about when we're passing around structs.
Clearly this won't work everywhere.

--
Andy Lester => an...@petdance.com => www.petdance.com => AIM:petdance

[Joshua Isom]

On Jun 18, 2007, at 4:48 PM, Andy Lester wrote:

> Is there a reason we use
>
> memcpy( dest, src, sizeof(FOO) );
>
> instead of
>
> *dest = *src;
>
> The latter should be the exact same code, but be much less likely to
> be screwed up.

No, they're extremely different. In the first, the data of FOO is
copied to dest, so dest can be modified without changing src. In the
second, src and dest point to the same data. If you modify one, all
are modified. If you want to clone something, or just move it to a new
location, you can't just set the pointer. If I'm missing something,
well maybe someone who knows more can provide more advice.

[Andy Armstrong]

On 19 Jun 2007, at 08:48, Joshua Isom wrote:
> On Jun 18, 2007, at 4:48 PM, Andy Lester wrote:
>
>> Is there a reason we use
>>
>> memcpy( dest, src, sizeof(FOO) );
>>
>> instead of
>>
>> *dest = *src;
>>
>> The latter should be the exact same code, but be much less likely
>> to be screwed up.
>
> No, they're extremely different. In the first, the data of FOO is
> copied to dest, so dest can be modified without changing src. In
> the second, src and dest point to the same data. If you modify
> one, all are modified.

Nonsense :)

> If you want to clone something, or just move it to a new
> location, you can't just set the pointer. If I'm missing
> something, well maybe someone who knows more can provide more advice.

You're thinking of pointer assignment. Andy is assigning the pointed-
to structures. Consider with ints instead of structs

int *p, *q;

memcpy(p, q, sizeof(int)); /* copy int at q into int at p */
*p = *q; /* the same */
p = q; /* p and q point to same memory */

You're thinking of the third case - Andy is doing the second.

--
Andy Armstrong, hexten.net

[Leopold Toetsch]

Am Montag, 18. Juni 2007 23:48 schrieb Andy Lester:
> Is there a reason we use
>
>    memcpy( dest, src, sizeof(FOO) );
>
> instead of
>
>    *dest = *src;
>
> The latter should be the exact same code, but be much less likely to  
> be screwed up.

I'm using a lot of the first kind. The main reason is documenting the bulk
copy operation (which is basically wrong - that's a docu thingy, if even).

Using 'sizeof(*dest)' is slightly better because the probability of
overwriting too small dest areas is minimized - not the correctness of the
code.

As Andy said: the best thing is using...

*dest = *src;

... which will fail (IIRC) on all incompatible pointers. Compilers of course
know about the size of the involved structure and can optimize the (possible
generated) memcpy to unrolled item copies.

There are 2 addendums though:

a) as that construct seems not to be known widely I'd write it like:

*dest = *src; /* copy contents of structure FOO */

or some such.

b) sometimes inside parrot guts only parts of structures are copied to
compatible pointers, which still will need the memcpy(3) call.

leo

[Allison Randal]

I wasn't entirely happy with either option, so I asked around a bit for
other ideas. I like the macro approach below, it gives us both self
documentation and better checking for the size of the two structs.

Allison

-------- Original Message --------
From: Nick Forrette <nfor...@wgz.com>
To: Allison Randal <all...@perl.org>

This can be a bit brittle -- you have to be careful when changing the
type of dst else you could end up with over or underflow.
memcpy(dst, src, sizeof(FOO));

I do like memcpy because it serves as a red flag that a potentially
medium to large size hunk of memory will be copied here,
memcpy(dst, src, sizeof(*dst));

This gets you better type checking (if src and dst are different
struct types, the compiler will complain). Much harder to overflow dst
or only partially copy src.
*dst = *src;

Something like this can get you the best of the above, as long as
you're careful about size-effects (I'd have to double check to be
sure, but I believe there are differences with pre-increment/decrement
in a dereference/assignment v.s. two arguments to a function).

#define copy_struct(dst, src) \
(*(dst) = *(src))

copy_struct(dst, src);

You could use a more robust macro that does a run time check for size
effects:

// returns int so it can be used in a , expression
int myAssert(int thisIsABool)
{
assert(thisIsABool);
return 0;
}

#ifdef NDEBUG
#define macro_assert(thisIsABool) 0
#else
#define macro_assert(thisIsABool) myAssert(thisIsABool)
#endif

#define copy_struct(dst, src) \
(\
macro_assert((dst) == (dst)), \
macro_assert((src) == (src)), \
(*(dst) = *(src)) \
)

Gives you what I consider the best of all worlds. The compiler will
flag copying between different struct types, it is more robust in the
face of changed types for src or dst, in form that clearly spells out
what you're trying to do, that at least gives you run time checking
for abuse of side effects in debug builds.

Nick

[Andy Lester]

On Jun 20, 2007, at 11:50 AM, Allison Randal wrote:

> I wasn't entirely happy with either option, so I asked around a bit
> for other ideas. I like the macro approach below, it gives us both
> self documentation and better checking for the size of the two
> structs.

I guess I don't see the need to document a standard C behavior with a
macro. We don't have

#define inc(x) (x)++

do we?

xoxo,
Andy

[Mark J. Reed]

On 6/20/07, Andy Lester <an...@petdance.com> wrote:
> I guess I don't see the need to document a standard C behavior with a
> macro. We don't have
>
> #define inc(x) (x)++
>
> do we?

Incrementing a var is much less likely to have unpredictable effects
due to modifying the wrong memory. Sure, x might be a pointer, and
things might gang agley there, but pointers getting set to the wrong
type of pointee is a pretty common problem, and one that I'm happy to
have some runtime support in locating.

--
Mark J. Reed <mark...@mail.com>

[Andy Lester]

On Jun 20, 2007, at 1:05 PM, Mark J. Reed wrote:

> Incrementing a var is much less likely to have unpredictable effects
> due to modifying the wrong memory. Sure, x might be a pointer, and
> things might gang agley there, but pointers getting set to the wrong
> type of pointee is a pretty common problem, and one that I'm happy to
> have some runtime support in locating.

My point is that

*d = *s;

is no more a weird cryptic construct than

n++;

or

while (*s)
*d++ = *s++;

for that matter.

[Mark J. Reed]

On 6/20/07, Andy Lester <an...@petdance.com> wrote:

> My point is that
>
> *d = *s;
>
> is no more a weird cryptic construct than
>
> n++;
>
> or
>
> while (*s)
> *d++ = *s++;
>
> for that matter.

That's a very good point. As you said in the original message, all you need is

*dest = *src;

since the compiler will automatically complain if the two pointers
don't point to the same type of object. AFAICT, the wrapping around
the assignment in the macro just makes sure that there aren't any side
effects - but the only reason side effects would be a problem is that
it's a macro...

[Allison Randal]

Andy Lester wrote:
>
> I guess I don't see the need to document a standard C behavior with a
> macro.

If you had read all the way through the message, you would see that the
biggest benefit is the ability to hang debugging hooks off the macro.
We have a number of these kinds of debugging hooks throughout Parrot
(src/gc/memory.c is a good example).

Allison

[Andy Lester]

No, I saw that, too. My point was just about documentation.

If we're set on using a macro for doing struct copies, then it needs
to handle copying multiple structs, since there a bunch of places
we're doing memcpy()s with math for multiple structs. I'll work on
that today. Getting rid of the math may be the best reason to use a
macro yet.