memcpy() bug

[Robert Pope]

I'm trying to add the structure for my board position at the leaf node to my PV structure, and I'm just not getting the copy correct. Can anyone help me out with what I am doing wrong?

Code: Select all

The structures are:

typedef struct {
   bitboard hash;
   bitboard bb_pieces[2][7];
   char kingsq[2];
   char board[64];  // bits 0-3 piecetype, bit 4 color
   int pieceCnt[2][7]; // number of pieces of each type
   int epSquare;
   char castleStat;
   char movingColor;
   char fiftyCount; 
   int moveCount;
   int mob[2];
} position;

typedef struct tagLINE {
    int cmove;            // Number of moves in the line
    int argmove[MAXPLY+1];  // PV line
	position leaf;		// Leaf position of PV
	int leafScore;
  } LINE;

and the code:

int Quiesce(position* bd,tree* space, int alpha,int beta,LINE* pline)
{
...
	// Assume this will be a leaf - copy the position and score;
	memcpy(&pline->leaf,&bd,sizeof(bd));
	Print(3, "Leaf Board\n");
	PrintBoard(bd);
	Print(3, "Copied to Pline\n");
	PrintBoard(&pline->leaf);
...
}

and the second PrintBoard call returns garbage.

[wgarvin]

bd is a pointer, its size is 4 or 8 bytes.

You probably meant to copy sizeof(*bd) bytes--or sizeof(Position).

(Edit: also you were copying from "address of the pointer bd" rather than "address pointed to by bd")

Code: Select all

memcpy(&pline->leaf,bd,sizeof(*bd));

[Robert Pope]

Thanks - I think those fixes did it. Sometimes I hate working with structures and pointers.

[sje]

Since source and destination objects are both structs of the same type, why not use a direct assignment instead of memcpy()?

Code: Select all

pline->leaf = *bd;

[wgarvin]

A trick I find useful, is to look at each type and expression one at a time, and mechanically translate the types into English. The important thing is to make sure to translate what the code actually says (instead of just saying what you intended it to mean when you wrote it)

position leaf;

memcpy(&pline->leaf, ...

"Address of a position"

int Quiesce(position* bd,

memcpy(..., &bd, ...);

"Address of ...a pointer to a position"

(oops!)

This trick is kind of like Rubber duck debugging, but for a single expression at a time.

Mechanically translating an expression (or its C type) into English, out loud, catches those cases where you took the address of the wrong thing or had too few/too many dereferences somewhere in the expression. And if you have a case where the expression is too complicated to easily translate it into English, that's a good clue that you should maybe break it down into simpler parts, with some named temporary variables or something.

[Robert Pope]

Since source and destination objects are both structs of the same type, why not use a direct assignment instead of memcpy()?

Code: Select all

pline->leaf = *bd;

Does that work? I thought you could only do that with the simple types, like int.

[sje]

Give it a try.

[kbhearn]

Since source and destination objects are both structs of the same type, why not use a direct assignment instead of memcpy()?

Code: Select all

pline->leaf = *bd;

Does that work? I thought you could only do that with the simple types, like int.

pointers are a simple type like ints essentially - it's what they point to that can be complex. the compiler prevents you from assigning different types of pointers to each other without casts, but as long as it's pointers of the same type, it's just like working with an int.

one caveat to assigning a pointer in such a way is if the source object has a different owner than the destination object and the owner may destroy it before the destination is done with it. i.e. assigning the address of a local variable to something that is intended to last beyond the scope of the current function = memory corruption waiting to happen. other examples would be when two different objects in some way both get a pointer to the same address and when one of them gets deleted it goes to cleanup its pointers despite there being other things still pointing to it.

The complications to these cases are why some people like smart pointer classes and shared pointer classes in c++ that either a) ensure only one variable can ever have a valid pointer to an address (assignment destroys the souce's reference) or b) keep track of how many times the object pointed to is referenced. These come with their own headaches though. Making a copy of any object that the function has a pointer to but doesn't 'own' is not a bad practice as long as it's not performance critical (sometimes what's being pointed to can be a rather large object and it's worth thinking about if you need to make a copy or can skip it - sometimes it's not much bigger than the pointer itself and you might just copy it to avoid headache).

[Dann Corbit]

Since source and destination objects are both structs of the same type, why not use a direct assignment instead of memcpy()?

Code: Select all

pline->leaf = *bd;

Does that work? I thought you could only do that with the simple types, like int.

If you have pointers to objects in your struct, then you might have problems due to shallow copy.

Do a web search for shallow copy deep copy if you are not sure what I mean.

[sje]

Since source and destination objects are both structs of the same type, why not use a direct assignment instead of memcpy()?

Code: Select all

pline->leaf = *bd;

Does that work? I thought you could only do that with the simple types, like int.

If you have pointers to objects in your struct, then you might have problems due to shallow copy.

Do a web search for shallow copy deep copy if you are not sure what I mean.

Objects (i.e., struct/class instances) can be divided into lightweight and heavyweight categories. A lightweight object can be copied with memcpy() or with a direct assignment, and everything should work. A heavyweight object will have at least one pointer to a dynamically allocated other object, and a simple copy might not work if later the same referenced object is deallocated twice.

Copying heavyweight objects is handled in C++ by means of what's called a copy constructor, a routine also known as a c'tor. The c'tor knows how to duplicate referenced objects as needed and even knows not to try and copy an object onto itself. Also in C++ is the struct/class destructor routine (or d'tor) which properly terminates an object, releasing allocated references as needed. Nearly all of the time the c'tor and d'tor routines are called automatically as needed.

A good C++ reference manual, much used by me: http://www.cplusplus.com/

----

And now for a programming trick of sorts. In Symbolic, the Position class is heavyweight, as are several of its components (various lists). Its copy constructor works by copying the simple stuff directly and using the copy constructors of its heavyweight components.

One of the Position methods is the Flip() routine which flips not only the colors and board of the position, but also the position's total move history, status lists, signatures, etc. A lot of work, but simply done because one of the Position members is the ifen string, the FEN of the Position instance initial set up. A Flip() works by creating a new Position instance using the flipped ifen string, and then playing all of the flipped moves from the source position's history.