Solved

memcpy a struct to an array

Posted on 2009-04-01
18
585 Views
Last Modified: 2012-05-06
I'll try to keep this as short as I can. I am basically implementing a mailbox system, which basically can store (send/receive) any type of messages, which can be a struct, char, int.. or whatever. However just now I realize that it doesn't work for sending over a struct. I've attached my mailbox code and as well as the test code I have. I believe the problem is with memcpy... but please let me know what I should do to fix it.

Some information regarding the code:
1. arg->arg3 is equal to 40 here, I got that from sizeof(DiskStruct), I don't know why it's 40
2. arg->arg2 here is the pointer to the DiskBuffer ds
3. boxToSend->box[i] is where I want to copy the struct to, it's basically just some space on the
    memory. I've malloced this before, I just didn't show it on the code below. This will be used to read back from the  
    mailbox. The size of box[i] is set to be bigger than the size of the struct as I don't know what kind of data will be
    put there, it's up to the user who uses the mailbox
4. In the struct I have void* buffer, this is basically because I can put/send anything to the
    mailbox, a char, struct, or whatever I want. Therefore I don't know the type and how big it
    will be so I declared it as a void*

The problem is actually because after I did the memcpy below and tried to read the struct back at boxToSend->box[i], the result I get is totally different. I tried to extract the value of unit, track, first, etc and it prints out some random weird numbers.  Which implies that the memcpy doesn't work. Please help me on this. And let me know if you need further information


typedef struct {

	P1_Semaphore sem;

	int unit;

	int track;

	int first;

	int sectors;

	int action; /* DISK_READ for read, DISK_WRITE for write */

	void* buffer;

} DiskStruct;

 

DiskStruct* ds = (DiskStruct*) malloc(sizeof(DiskStruct));

ds->sem = P1_SemCreate(0);

ds->unit = unit;

ds->track = track;

ds->first = first;

ds->sectors = sectors;

ds->action = USLOSS_DISK_WRITE;

ds->buffer = buffer;

 

 

 

/*ACTUAL CODE*/

 

memcpy((void*) &(boxToSend->box[i]), arg->arg2, arg->arg3);

Open in new window

0
Comment
Question by:kuntilanak
  • 9
  • 7
18 Comments
 
LVL 40

Expert Comment

by:mrjoltcola
Comment Utility
I don't think you posted enough code for me to help. So far it looks ok, albeit dangerous, since you admit DiskStruct->buffer can point to anything, so I'm not sure how you can tell what it points to unless you save the size in the mailbox as well. But that might be a different issue.

Post more code?
0
 

Author Comment

by:kuntilanak
Comment Utility
here's some more code.. I hope you understand, as I don't know where to start explaining.. USLOSS_Args is just a struct.. The arg is just as I explained above for the send, so I hope you can trace it back
void MBoxCreate(USLOSS_Sysargs* arg)

{

	if (numboxes != P2_MAX_MBOX) {

		MBox* newBox = (MBox*) malloc(sizeof(MBox));

		newBox->sender = P1_SemCreate(1);

		newBox->receiver = P1_SemCreate(0);

		newBox->receipt = P1_SemCreate(0);

		newBox->numslots = arg->arg1;

		newBox->size = arg->arg2;

		newBox->numMessages = 0;

		newBox->flag = -1;
 

		/* CREATE THE MAILBOX */

		void** newArray = (void**) malloc(newBox->size * newBox->numslots);

		int i;
 

		newBox->box = newArray;
 

		for (i = 0; i < P2_MAX_MBOX; i++)

			if (MBoxes[i] == NULL)

				break;
 

		MBoxes[i] = newBox;
 

		if (arg->arg1 < 0 || arg->arg2 < 0)

			arg->arg4 = -1;

		else

			arg->arg4 = 0;
 

		arg->arg1 = i;

		numboxes++;
 

		newBox->messSizes = (int*) malloc(sizeof(int) * newBox->numslots);

	} else {

		arg->arg1 = -1;

	}
 

}
 
 

void MBoxSend(USLOSS_Sysargs* arg)

{

	int id = (int) arg->arg1;

	if (id < 0 || id >= P2_MAX_MBOX || MBoxes[id] == NULL) {

		arg->arg4 = -1;

		return;

	}
 

	MBox* boxToSend = MBoxes[id];
 

	if (arg->arg3 > boxToSend->size) {

		arg->arg4 = -1;

		return;

	}
 

	P1_P(boxToSend->sender);
 

	// send the message

	int i = 0;

	for (; i < boxToSend->numslots; i++)

		if (boxToSend->messSizes[i] == 0) {

			break;

		}
 
 

	/* CHECK IF MB IS FULL */

	if (i == boxToSend->numslots) {

		P1_V(boxToSend->receiver);

		if (boxToSend->flag == 1) {

			P1_V(boxToSend->receipt);

		}

		boxToSend->flag = 0;

		P1_P(boxToSend->receipt);

		for (i = 0; i < boxToSend->numslots; i++)

			if (boxToSend->messSizes[i] == 0) {

				break;

			}

	} else {

		P1_V(boxToSend->receiver);
 

	}
 

	memcpy((void*) &(boxToSend->box[i]), arg->arg2, arg->arg3);
 

	

	boxToSend->messSizes[i] = arg->arg3;

	arg->arg2 = (void*) boxToSend->messSizes[i];

	arg->arg4 = 0;

	boxToSend->numMessages++;
 

	P1_V(boxToSend->sender);

	if (boxToSend->flag == 1) {

		P1_V(boxToSend->receipt);

		boxToSend->flag = -1;

	}
 

}

Open in new window

0
 

Author Comment

by:kuntilanak
Comment Utility
The problem might be in here:

void** newArray = (void**) malloc(newBox->size * newBox->numslots);

that's actually where the actual mailbox is and where I actually send/do a memcpy if I send something to the mailbox.

I declared it as a void** as I don't know what type of array that is, besides that array could contain different kind of things, index 0 can be a struct, index 1 can be an int, etc..etc.. so that's why it's a void**
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
Comment Utility
>>>> memcpy((void*) &(boxToSend->box[i]), arg->arg2, arg->arg3);

What has this statement to do with the DiskStruct structure above?

>>>> ds->buffer = buffer;

You prrobably need to copy the buffer to newly allocated memory rather than assigning a pointer. That would mean you either need to know the max size of the buffer or have to add an additional member which specifies the size of the buffer:

(A)

typedef struct {
      P1_Semaphore sem;
      int unit;
      int track;
      int first;
      int sectors;
      int action; /* DISK_READ for read, DISK_WRITE for write */
      unsigned char buffer[MAX_BUF_SIZE];
} DiskStruct;

or

(B)

typedef struct {
      P1_Semaphore sem;
      int unit;
      int track;
      int first;
      int sectors;
      int action; /* DISK_READ for read, DISK_WRITE for write */
      void* buffer;
                int      bufsiz;
} DiskStruct;



For (A) you would have

    memcpy(ds->buffer, buffer, min(MAX_BUF_SIZ, sizBuf));

where sizBuf would be an additional argument passed to your function with buffer.

For (B) you would have


    ds->buffer = (unsigned char*)malloc(sizBuf);
    memcpy(ds->buffer, buffer, sizBuf);

where gain sizBuf would be an additional argument passed to your function with buffer.
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
Comment Utility
>>>> void** newArray = (void**) malloc(newBox->size * newBox->numslots);
Here you were allocating an array of pointers (only), but not the memory for the data.  But the 'newBox->size * newBox->numslots´' expression seems to be the size of data and not the number of slots times size of pointer.

I wonder why you want to go the stony way of dynamic allocation when you are so much unexperienced in pointers and allocation issues. For example the

    memcpy((void*) &(boxToSend->box[i]), arg->arg2, arg->arg3);

most probably would crash as I can't see a proper allocation for boxToSend->box[i].

You better would use fixed-sized arrays and structures on the stack which are much easier to handle.

typedef struct
{
     int length;
     int senderId;
     int receiverId;
     int status;      
     unsigned char message[MAX_MSG_SIZ];
} MailBoxSlot;

typedef struct
{
     int numslotsUsed;
     int ownerId;
     MailBoxSlot[MAX_SLOTS];
} MailBox;

then with

   MailBox mbox = { 0 };

you have a fully allocated mailbox with all members set to zero.

BTW, it seems it is a .cpp source (or you wouldn't be able to create new variables in the middle of a scope). Why don't using more comfortable and safer C++, e, g, container classes or string class?



   
0
 

Author Comment

by:kuntilanak
Comment Utility
if I do it your way above with the structs and array... how can I say copy the diskstruct to the array mailbox?
0
 

Author Comment

by:kuntilanak
Comment Utility
>>BTW, it seems it is a .cpp source (or you wouldn't be able to create new variables in the middle of a >>scope). Why don't using more comfortable and safer C++, e, g, container classes or string class?

I am, working on C, not C++
0
 

Author Comment

by:kuntilanak
Comment Utility
>>But the 'newBox->size * newBox->numslots´' expression seems to be the size of data and not the >>number of slots times size of pointer.

is that how C interprets it? I was thinking that if I did a void** that would mean to be an array of (void)* objects...

is there a way to fix this without using the struct approach you showed us, as I'll need to change a lot of stuffs that way... I already have an array of mailboxes, so treating each slot as a struct and a mailbox as a struct would make it more complex
0
Top 6 Sources for Identifying Threat Actor TTPs

Understanding your enemy is essential. These six sources will help you identify the most popular threat actor tactics, techniques, and procedures (TTPs).

 
LVL 39

Accepted Solution

by:
itsmeandnobodyelse earned 500 total points
Comment Utility
>>>> as I'll need to change a lot of stuffs that way...
But your mallocs don't work in the moment and as you have not posted the structures it is impossible to say where you missed an allocation and where not. And a void** is nothing where I could make any analysis from.

My structure was only a sample. I would suggest to take your structure(s) and exchange pointer members by fixed-sized arrays (taht way you corrected the biggest flaws and could use all following statements (beside of malloc).

Instead of an array of pointers use a fixed-sized array of structures. You only have to change the arrow -> by a period .

If you need to pass the structure to a function use the & to pass it as pointer.

>>>> I already have an array of mailboxes

No, you have an array of unitialized pointers pointing to nowhere. The malloc only reserves space for the pointers not for the data.

>>>>  I was thinking that if I did a void** that would mean to be an array of (void)* objects...

indeed, but a void* object is a pointer and nothing else. Worse, it is an uninitialized pointer and you can be happy if it is NULL by accident. The malloc doesn't know what you have left of the assignment operator. It simply allocates memory for the size you pass and returns the start address of that memory. If you request

     newBox->size * newBox->numslots

memory and the size is 64 and numslots is 100 you got 6400 bytes allocated. Dividing that by pointer size it is 1600 for 32bit pointers and 800 for 64bit pointers. Let us assume the latter, then you may access

 newArray[0] to newArray[799]

without accessing beyond the allocation made by malloc. I assume you don't actually need 800 slots for your mailbox but only numslots slots. Then it would have been

   void** newArray = malloc(newBox->numslots * sizeof(void*));

And to get an allocation for each slot you need

   for (i = 0; i < newBox->numslots; ++i)
         newArray[i] = malloc(newBox->size);  

if the size is in bytes. But the slots still were not initialized. So it is

   for (i = 0; i < newBox->numslots; ++i)
   {
         newArray[i] = malloc(newBox->size);  
         memset(newArray[i], 0, newBox->size);
    }

or using calloc instead of malloc.

You could get rid of all that by using a fixed sized array of your structure for one slot. And it automatically would be zero if you do like

   MailboxSlot   newArray[MAX_SLOTS]  = { 0 };

Define it at top of your function and get rid of the malloc.

   



0
 

Author Comment

by:kuntilanak
Comment Utility
ok, I've changed my code of initializing the array to the following:

however now about having problems to copy the buffer to the struct, I wanted to use option B that you showed on the second post. However how do I get the bufSize, below is part of the function signatures I have


/*code*/

int P2_DiskWrite(int unit, int track, int first, int sectors, void* buffer)

{

	if (unit < 0 || unit >= USLOSS_DISK_UNITS)

		return -1;
 

	DiskStruct* ds = (DiskStruct*) malloc(sizeof(DiskStruct));

	ds->sem = P1_SemCreate(0);

	ds->unit = unit;

	ds->track = track;

	ds->first = first;

	ds->sectors = sectors;

	ds->action = USLOSS_DISK_WRITE;

	ds->buffer = malloc(/*what is the size supposed to be here*/);

	memcpy(ds->buffer, buffer, /*what is the size of void* */);

        ................................

/**********************/
 
 

void** newArray = (void**) malloc(sizeof(void*) * newBox->numslots);

int i;

for (i = 0; i < newBox->numslots; i++){

	newArray[i] = malloc(sizeof(newBox->size));

	memset(newArray[i], 0, newBox->size);

}

Open in new window

0
 

Author Comment

by:kuntilanak
Comment Utility
ok I think I got it to work now, however I need to understand what this is... so void** is an array of void*, and then when I do the:

newArray[i] = malloc(sizeof(newBox->size));

for each slots I have, is this basically allocating space for how big each slot is, newArray[i] type is a void* right? and then when I do a memcpy(newArray[i] , ds, structsize);

am I actually copying a pointer to another pointer, and not actually copying the real value that ds is pointing to? or am I actually copying the value (i.e the struct) to newArray[i] ?? If I am just copying a pointer to another place then, I'd say this is useless right as the data pointed by the pointer might be changed and the results when I tried to receive from would be different from what I originally send.
I hope this makes sense... I know I am not experienced enough with pointers, especially with void*, but I am trying  to understand what I am doing here and I hope what I am doing does what I want to do.. which is copying the actual data (struct) to the array slots
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
Comment Utility
>>>> newArray[i] type is a void* right?

Yes, a pointer to unknown (or arbitrary) data.

>>>> then when I do a memcpy(newArray[i] , ds, structsize);

Then, you were copying data ...

   newArray[i] = ds;

would be copying the pointer.

>>>> I know I am not experienced enough with pointers, especially with void*

You must not use void* or even void**

If you have a structure which maps your mailbox slot, say  typedef struct {  ... } MailboxSlot;

Then you could/should do

   MailboxSlot* newArray = (MailboxSlot*)malloc(newBox->numSlots*sizeof(MailboxSlot*));

That is much better than to using a void** which actually is nothing else but a 'unsigned char**' a pointer to an array of pointers to byte arrays, or less. You can every array of pointers look at as an array of pointers to sequences of bytes but actually it doesn't help much if you couldn't decide. It only prevents the compiler from being able to check type integrity and hence it is less secure and more error-prone. Note, the void* type is much less then a baseclass pointer in C++. It actually is only for defining an argument type so that the argument can take any (address of) data. Using a void* for non argument types simply is bad programming.  
0
 

Author Comment

by:kuntilanak
Comment Utility
ok.. I am going to bear with the void** as of now.. as it already works and I thought of it that it would need to take a lot of work to change it to the struct implementation you said... also say that  newArray[i]  is already occupied by something and I want to use it for some other storage do I need to free it first?
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
Comment Utility
>>>> also say that  newArray[i]  is already occupied by something and I want to use it for some other storage do I need to free it first?

Yes for any malloc you need a free. If you allocated memory in a loop for each slot you need to free it in a loop again.
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
Comment Utility
Why did you give a C grade?

A C grade is appropriate when you got no (more) answers though the initial question was not yet answered.

A B grade is appropriate if you got valid answers but there either were things left open or you had to work out most parts of the solution yourself.

An A grade should be given if one or more comments do answer the initial question sufficiently.

So, here you could choose between B or A as you got a lot of answers even if you did not like them all.
0
 

Author Comment

by:kuntilanak
Comment Utility
I am sorry, I didn't mean to give you a C grade.. I meant to give you A since the first time as you explained everything clearly.. must have pressed the wrong button. I am sorry once again, however I just want to let you know that I really appreciate your help.. how can I change it to A now
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
Comment Utility
You could check the 'Request Attention' Link at the top of that thread and the bottom of your question.

That way you can get a moderator to reopen the question.

Thanks for responding. I already assumed that it happened by accident.

Regards
0

Featured Post

How to run any project with ease

Manage projects of all sizes how you want. Great for personal to-do lists, project milestones, team priorities and launch plans.
- Combine task lists, docs, spreadsheets, and chat in one
- View and edit from mobile/offline
- Cut down on emails

Join & Write a Comment

An Outlet in Cocoa is a persistent reference to a GUI control; it connects a property (a variable) to a control.  For example, it is common to create an Outlet for the text field GUI control and change the text that appears in this field via that Ou…
This is a short and sweet, but (hopefully) to the point article. There seems to be some fundamental misunderstanding about the function prototype for the "main" function in C and C++, more specifically what type this function should return. I see so…
The viewer will learn how to pass data into a function in C++. This is one step further in using functions. Instead of only printing text onto the console, the function will be able to perform calculations with argumentents given by the user.
The viewer will learn how to clear a vector as well as how to detect empty vectors in C++.

771 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

10 Experts available now in Live!

Get 1:1 Help Now