Array setup Question

J

Jake Thompson

Hello

I have the following defined structure

struct cm8linkstruc
{
char *type; /* type of item*/
char *desc; /* description of item */
char *item_increment; /* increment value for item in
folder */
char *itemid; /* id of returned item */
int count; /*total count of array */
};


In the function I have this

struct cm8linkstruc **cm8link;

If I want to allocate 50 occurrences of this array then if the
following correct? It appears to be correct at least at a compile level
because it compiles

/*Allocate the array for use over in the dll */
cm8link = (struct cm8linkstruc **) malloc(50 * sizeof(struct
cm8linkstruc *));
for(h = 0; h< 50 ; h++)
{
cm8link[h] = (struct cm8linkstruc *) malloc(sizeof(struct
cm8linkstruc));
cm8link[h]->type = 0;
cm8link[h]->desc = 0;
cm8link[h]->item_increment = 0;
cm8link[h]->itemid = 0;
cm8link[h]->count = 0;
}

I have a function in a dll that will add values to the array and then
pass the populated array back to the calling exe

If I am on the right track then would I pass &cm8link to my function in
my dll in order to use it to populate it before I return from the
called function?

Thanks
Jake
 
A

Artie Gold

Jake said:
Hello

I have the following defined structure

struct cm8linkstruc
{
char *type; /* type of item*/
char *desc; /* description of item */
char *item_increment; /* increment value for item in
folder */
char *itemid; /* id of returned item */
int count; /*total count of array */
};


In the function I have this

struct cm8linkstruc **cm8link;

If I want to allocate 50 occurrences of this array then if the
following correct? It appears to be correct at least at a compile level
because it compiles

/*Allocate the array for use over in the dll */
As soon as you say `dll', it's either irrelevant -- or off topic in this
newsgroup.
cm8link = (struct cm8linkstruc **) malloc(50 * sizeof(struct
cm8linkstruc *));
Better:
cm8link = malloc(sizeof *cm8link * 50);

Since malloc() returns a pointer to void and such pointer is convertible
to any pointer to object the cast is unnecessary -- and should be
avoided (it can hide, for example, the failure to include <stdlib.h>.

Further, using the `sizeof object' form is often preferable to the
`sizeof(type)' form; it the type happens to be changed, it makes for one
less change that need be made...
and, of course, make sure the malloc() succeeded.
for(h = 0; h< 50 ; h++)
{
cm8link[h] = (struct cm8linkstruc *) malloc(sizeof(struct
cm8linkstruc));
Better:
cm8link[h] = malloc(sizeof **cm8link);
and, of course, make sure the malloc() succeeded
cm8link[h]->type = 0;
cm8link[h]->desc = 0;
cm8link[h]->item_increment = 0;
cm8link[h]->itemid = 0;
cm8link[h]->count = 0;
}

I have a function in a dll that will add values to the array and then
pass the populated array back to the calling exe

If I am on the right track then would I pass &cm8link to my function in
my dll in order to use it to populate it before I return from the

Again either this `dll' stuff is irrelevant or off topic.
called function?
HTH,
--ag
 
B

Ben C

Hello

I have the following defined structure

struct cm8linkstruc
{
char *type; /* type of item*/
char *desc; /* description of item */
char *item_increment; /* increment value for item in
folder */
char *itemid; /* id of returned item */
int count; /*total count of array */
};


In the function I have this

struct cm8linkstruc **cm8link;

/*Allocate the array for use over in the dll */
cm8link = (struct cm8linkstruc **) malloc(50 * sizeof(struct
cm8linkstruc *));
for(h = 0; h< 50 ; h++)
{
cm8link[h] = (struct cm8linkstruc *) malloc(sizeof(struct
cm8linkstruc));
cm8link[h]->type = 0;
cm8link[h]->desc = 0;
cm8link[h]->item_increment = 0;
cm8link[h]->itemid = 0;
cm8link[h]->count = 0;
}

This looks all right, although you should do something if the
allocations fail, and it isn't necessary to cast the return of malloc.

Make sure you free the whole lot in the right way too when the time
comes.
I have a function in a dll that will add values to the array and then
pass the populated array back to the calling exe
If I am on the right track then would I pass &cm8link to my function
in my dll in order to use it to populate it before I return from the
called function?

It sounds like &cm8link is one too many levels of indirection than you
need. Could you not just pass cm8link?

Then the function in the dll populates the array like this:

for (i = 0; i < 50; i++)
{
cm8link->desc = something;
...
}
 
B

Ben C

struct cm8linkstruc
{
char *type; /* type of item*/
char *desc; /* description of item */
char *item_increment; /* increment value for item in
folder */
char *itemid; /* id of returned item */
int count; /*total count of array */
};


In the function I have this

struct cm8linkstruc **cm8link;

/*Allocate the array for use over in the dll */
cm8link = (struct cm8linkstruc **) malloc(50 * sizeof(struct
cm8linkstruc *));
for(h = 0; h< 50 ; h++)
{
cm8link[h] = (struct cm8linkstruc *) malloc(sizeof(struct
cm8linkstruc));
cm8link[h]->type = 0;
cm8link[h]->desc = 0;
cm8link[h]->item_increment = 0;
cm8link[h]->itemid = 0;
cm8link[h]->count = 0;
}

I just said this "looked all right", but is there actually any reason
why you need an array of 50 pointers to individually allocated structs?

A typical reason to do that is if the "structs" are of variable sizes
that are determined later. But here they're not, they're all just sizeof
(cm8linkstruc).

So why not just allocate one array?

struct cm8linkstruc *cm8links =
malloc(50 * sizeof (struct cm8linkstruc));

for (h = 0; h < 50; h++)
{
cm8links[h].type = 0;
/* etc... */
}
 
F

Flash Gordon

Jake said:
Hello

I have the following defined structure

struct cm8linkstruc
{
char *type; /* type of item*/
char *desc; /* description of item */
char *item_increment; /* increment value for item in
folder */
char *itemid; /* id of returned item */
int count; /*total count of array */
};


In the function I have this

struct cm8linkstruc **cm8link;

If I want to allocate 50 occurrences of this array then if the
following correct? It appears to be correct at least at a compile level
because it compiles

It would be easier if you provided a complete compilable example. For
example, I don't know whether you failed to include stdlib.h (which
without any other declaration for malloc in scope would give you
undefined behaviour) or merely failed to include it in your snippet.

#include said:
/*Allocate the array for use over in the dll */
cm8link = (struct cm8linkstruc **) malloc(50 * sizeof(struct
cm8linkstruc *));

*Never* cast the return value of malloc. See
http://c-faq.com/malloc/mallocnocast.html

You should also check the rest of the FAQ it has a lot of useful material.

cm8link = malloc(50 * sizeof *cm8link);

The above is much simpler, and saves having to count stars to see if you
got it right.

You should also check the return value. malloc can fail.
for(h = 0; h< 50 ; h++)
{
cm8link[h] = (struct cm8linkstruc *) malloc(sizeof(struct
cm8linkstruc));

See above.
cm8link[h] = malloc(sizeof *(cm8link[h]));

Again, check to see if malloc succeeded.
cm8link[h]->type = 0;
cm8link[h]->desc = 0;
cm8link[h]->item_increment = 0;
cm8link[h]->itemid = 0;
cm8link[h]->count = 0;
}

I have a function in a dll that will add values to the array and then
pass the populated array back to the calling exe

If I am on the right track then would I pass &cm8link to my function in
my dll in order to use it to populate it before I return from the
called function?

Since you don't give a complete compilable example I'm not entirely sure
what you are trying to do. However, perhaps you should read
http://c-faq.com/ptrs/passptrinit.html

In fact, you should probably start off by reading all of sections 3, 6
and 7 of the FAQ. You will probably find all of your answers there.
 
J

Jake Thompson

Hello guys I made the following changes to my code based upon what I
read and the input that you guys gave me

my structure with all char * values

struct cm8linkstruc
{
char *type; /* type of item*/
char *desc; /* description of item */
char *item_increment; /*increment value for item
in folder */
char *itemid; /* id of returned item */
};

struct cm8linkstruc cm8link;

Sample of code

/*Allocate the structure */
cm8link = malloc(sizeof cm8link * fcount);
if(!cm8link)
{
return 1;
}

for(h = 0; h< fcount ; h++)
{
cm8link[h] = malloc(sizeof (cm8link[h]));
cm8link[h]->type = 0;
cm8link[h]->desc = 0;
cm8link[h]->item_increment = 0;
cm8link[h]->itemid = 0;
}
l_stat = u_cm8_getfolditemmatch((PITEMID)cptr, &cm8link);
if(l_stat)
{
/*clean it structure and free it*/
for(h = 0; h< fcount ; h++)
{
u_free_uni(cm8link[h]->type);
u_free_uni(cm8link[h]->desc);
u_free_uni(cm8link[h]->item_increment);
u_free_uni(cm8link[h]->itemid);
free(cm8link[h]);
}
return(1);
}


Originally I was going to make the cm8linkstruc part of another
structure like this

struct cm8rcstruc
{
BOOLEAN checkedout; /* status of checked
out object*/
char *userid; /* user who has object
checked out */
short objtype; /* True is a folder
\False is not */
char *note; /* Returned note */
struct cm8linkstruc *u_cm8link; /*pointer to link
structure*/
};

That is why I had so many stars. I since changed my mind and just want
to make the cm8linkstruc it's own structure without being part of
another structure. By doing that I believe my indirection gets messed
up

so instead of defining the structure as
struct cm8linkstruc **cm8link;
I now define it as
struct cm8linkstruc cm8link;

which like I said messes up my indirection for my mallocs correct. How
can I fix this?

Also when doing the free of the structure I free each char * in the
array. This to me means that I have to allocate the char * in a
similar fashion which I am not currently doing because all I do on the
allocation is set everything to 0 when I think I should be doing
something more involved - Can someone show me an example?

Sorry for all the questions but I derailed very easy with allocations
and indirections and I am just trying to get back on track.

Thanks
Jake
 
K

Keith Thompson

Jake Thompson said:
Hello guys I made the following changes to my code based upon what I
read and the input that you guys gave me

my structure with all char * values

struct cm8linkstruc
{
char *type; /* type of item*/
char *desc; /* description of item */
char *item_increment; /*increment value for item
in folder */
char *itemid; /* id of returned item */
};

struct cm8linkstruc cm8link;

Ok, cm8link is a struct object.
Sample of code

/*Allocate the structure */
cm8link = malloc(sizeof cm8link * fcount);

You're assigning the result of malloc() (a pointer) to a structure
object. That doesn't make any sense.

You probably want cm8link to be a pointer to struct:

struct cm8linkstruc *cm8link;

(You might want a different name for it, but I'll leave the name
alone.)

You can then allocate an array of "fcount" structures like this:

cm8link = malloc(sizeof *cm8link * fcount);

Note carefully that it's "sizeof *cm8link", not "sizeof cm8link".
if(!cm8link)
{
return 1;
}

Good, you're checking whether the malloc() succeeded. I presume
returning 1 from whatever function this code is part of makes sense.
(If the code is in main() then (a) it probably shouldn't be, and (b)
"return 1;" is not a portable way to indicate an error; use
EXIT_FAILURE said:
for(h = 0; h< fcount ; h++)
{
cm8link[h] = malloc(sizeof (cm8link[h]));

If cm8link is a pointer, then cm8link[h] is a struct; again, you're
assigning a pointer value to a struct object, which doesn't make any
sense. (And you're not checking whether the malloc() succeeded.)

If you want to allocate "fcount" structures, just allocate them all as
an array. You could allocate an array of pointers, and then allocate
each structure individually, but there doesn't seem to be any reason
to do so.

Decide what you want to allocate, and how you want to allocate it.
Drawing pictures on paper with boxes and arrows can be helpful (boxes
represent structures, arrows represent pointers).
cm8link[h]->type = 0;
cm8link[h]->desc = 0;
cm8link[h]->item_increment = 0;
cm8link[h]->itemid = 0;

You're setting each char* member to 0, i.e., a null pointer. (I'd use
NULL rather than 0.)
}
l_stat = u_cm8_getfolditemmatch((PITEMID)cptr, &cm8link);

No idea what this does.
if(l_stat)
{
/*clean it structure and free it*/
for(h = 0; h< fcount ; h++)
{
u_free_uni(cm8link[h]->type);
u_free_uni(cm8link[h]->desc);
u_free_uni(cm8link[h]->item_increment);
u_free_uni(cm8link[h]->itemid);
free(cm8link[h]);

I have no idea what u_fre_uni does.
}
return(1);
}
[...]

malloc() can allocate either a single object or a contiguous array of
objects; you just need to tell it how many bytes to allocate. It's
important to have a mental picture of what your data structure looks
like; as I wrote above, paper diagrams can be helpful here. Always
check the result of malloc(). Your program needs to call free() once
for each call to malloc() (that doesn't necessarily mean that that
exactly one call to free() will appear in your source for each
malloc() call in your source, but it often does.)
 
B

Ben C

[...]
Also when doing the free of the structure I free each char * in the
array. This to me means that I have to allocate the char * in a
similar fashion which I am not currently doing because all I do on the
allocation is set everything to 0 when I think I should be doing
something more involved - Can someone show me an example?

If you call free with NULL, nothing happens, so this is OK, as it
stands. If you want to actually store data (strings probably) using
those char pointers, there will need to be some memory somewhere.

If you want to make those pointers point to strings somewhere else your
program, you may decide to make them point to newly allocated blocks
Sorry for all the questions but I derailed very easy with allocations
and indirections and I am just trying to get back on track.

It is difficult to understand exactly what things in particular it is
you're having problems with.

I would suggest get clear in your mind _first_ how things should be laid
out in memory and when they should be allocated and deallocated. Then
sort out the code that reads and writes them, because it won't be
difficult to make it work with whatever layout you decide on. I wonder
if your problem isn't that you're trying to build a data layout that
matches some other code you already have, but that could easily be
changed.

Here is a test program in case any part of it is any use:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

struct link
{
char *type;
char *name;
};

#define FCOUNT 4

int main(void)
{
int i;
struct link *p;

/* Allocate FCOUNT links in one array */
struct link *links = malloc(FCOUNT * sizeof (*links));
/* remember to check allocation */

/* Initially they have no name or type */
for (i = 0; i < FCOUNT; i++)
{
links.type = NULL;
links.name = NULL;
}

/*
* How to copy a string into one of the links' fields. You may not have
* strdup on your system, but you probably do.
*/
links[2].type = strdup("hello");
/* remember to check allocation */

/* How to get a pointer to an individual link */
p = links + 2;

printf("%s world\n", p->type);

/* Free the contents of any links that have contents. */
for (i = 0; i < FCOUNT; i++)
{
free(links.type); /* links.type may or may not be NULL */
free(links.name);
}

free(links);

return 0;
}

HTH
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
474,176
Messages
2,570,947
Members
47,498
Latest member
log5Sshell/alfa5

Latest Threads

Top