views:

105

answers:

4

Having considerable trouble with some pointer arithmatic. I think I get the concepts (pointer variables point to a memory address, normal variables point to data) but I believe my problem is with the syntax (*, &, (*), *(), etc.)

What I want to do is build dynamic arrays of a custom struct (i.e. arrays of pointers to heap structs), and my interface provides two methods, "ad_to_obj_array" (which takes the object to add and the array which can be null for empty) and "obj_array_dustbin" (which just takes the array to dispose, also disposing of the contents, the heap objs). The former is rendered below.

The details of the objects are not important (and the struct has been renamed anyway) but my solution to the general problem is below, and I'd be grateful if you can spot the error. The compiler is complaining about an invalid lvalue, where I try and assign the address in the pointer on the RHS to the pointer value in an array of pointers to heap structs:

#define NUM_ELEM(x) (sizeof (x) / sizeof (*(x)))

obj* add_to_obj_array(obj* new_obj, obj* array)
{
  int number_of_elements = 0;
  if (array != NULL)
  {
    number_of_elements = NUM_ELEM(array);
  }

  obj* new_array = NULL;

  /* note: I am expecting sizeof(new_obj) to return the size of an obj* 
     to go into the array of pointers. */
  if ( NULL ==
       (new_array = (obj*)malloc((number_of_elements + 1)* sizeof(new_obj))) )
  {
    /* memory request refused :( */
    return NULL;
  }

  /* copy the old array pointers into the new array's pointer slots: */
  int i;
  for (i = 0; i < number_of_elements; i++)
  {
    &(new_array[i]) = &(array[i]);
  }

  /* add the new item to the end (assign pointer value directly): */
  new_array[number_of_elements] = new_obj;

  if (number_of_elements > 0)
  {
    free(&array);
  }

  return new_array;
}

Now, I have tried the following permutations of the offending line:

  &(new_array[i]) = &(array[i]);
  *(new_array[i]) = &(array[i]);
  new_array[i] = &(array[i]);

and all give a compiler error of one sort or another. I am fairly sure that the right hand side is the address of the ith element of the old array, but how to I assign to the ith element of the new, when the elements of the array are pointers to structs?

EDIT - please note, the macro NUM_ELEM above DOES NOT WORK; it will always return 1. See @Merlyn Morgan-Graham's answer below for why.

A: 

Just assign the values across. new_array[i] = array[i].

The problem you may be running into is that, for obj* to actually be an array of pointers, obj must itself be a pointer type:

typedef struct
{
  int value1;
} obj_pool;

typedef obj_pool* obj;

int main(int argc, char* argv[])
{
  obj_pool pool1;
  pool1.value1 = 5;
  obj array[] = { &pool1 };
  array[0]->value1 = 16;
  return 0;
}

Another problem you'll run into once you get this compiling is that sizeof(array) == sizeof(obj*). NUM_ELEM(array) will always return the same value. This means you'll have to pass a size_t array_size parameter to your function.

Merlyn Morgan-Graham
BTW, I don't recommend you make this typedef =P I instead suggest that you use obj**, as is the common C parlance. See Jerry's answer. His diagrams are pretty nice.
Merlyn Morgan-Graham
why will NUM_ELEM fail? (I just grabbed it off the web, and as I said I haven't tested it yet..) surely if obj* blah was allocated with malloc to be an array pointer, when you call sizeof it returns the amount of memory that was allocated, rather than the size of the pointer itself? if not I'm going to need to use a persistant varaible (static, in c?) to keep track of the array size, which is much messier code.
tehwalrus
!! it would appear that you are correct, NUM_ELEM entirely fails to work. I'll have to keep track manually (grumble grumble).
tehwalrus
@tehwalrus: Not my ideal link for this concept, but - http://stackoverflow.com/questions/1975128/sizeof-an-array-in-the-c-programming-language. Basically, an array decays to a pointer when it is passed to a function. Ways to deal with this are: 1. Pass both the array, and a size parameter. 2. Have a sentinel value in the array (e.g. the last element of the array must always be set to `NULL`, similar to null-terminated character arrays being used as strings). 3. Pass a pointer to the first element in the array, and a pointer to one-past-the-last element, and use `current != end` in your loops
Merlyn Morgan-Graham
thanks, interesting point. I actually built a new struct obj_array containing a obj** "array" and an int "count"; and just have this method recieve the obj_array* and increment the count member at the end. If I can template/generecise it I'll pop it on the web somewhere, might also write a ruby script to generate "array" structs and methods based on a header file. we'll see how much time I have left at the end of the project! :)
tehwalrus
@tehwalrus: "template/genericize" - you're not talking c++, are you? If you were, I'd take a *totally* different approach to the implementation of this problem, if I were you. You could still have the same memory layout, but the code would look way different.
Merlyn Morgan-Graham
no, I was talking about looking up what scope C has for that kind of thing. If that is all just half-remembered stuff from c++ then sorry! I'll write a script that generates a proper C file for arbitrary structs instead.
tehwalrus
+1  A: 

You have two arrays doesn't new_array[i] = array[i] do what you need.

  • Have you looked at realloc as a possible solution.
rerun
+1, If the arrays are the same type, then this should always work. This assumes that you want to do a shallow copy of the pointers. If you want a deep copy (copy the underlying pools), you're going to have to do more than just assigning the pointers across.
Merlyn Morgan-Graham
It's an array of `obj` however, not `obj *`. I don't think this is tehwalrus' intention.
Jeff M
@Jeff M yes it is an array of the structs but if he wants a shallow copy it should work.
rerun
@rerun: Ultimately the problem was that the arrays were not the correct type based on the description. What you have fixes that line but doesn't address the real problem. Jerry addressed it well I think with tehwalrus' actual intention in mind.
Jeff M
Thanks, it looks like realloc will yeild some further optimisations. tip of the hat! :)
tehwalrus
+5  A: 

Based on your description, you're starting off wrong, so by the time you get to copying things, nothing you can do is likely to work.

Right now, you've defined new_array (and, presumably, array) as a pointer to obj. The result looks like this:

alt text

In this case, you have a pointer to a dynamically allocated array of objects. When/if you expand the allocation, you'll need to copy all the objects themselves.

According to your description: "(i.e. arrays of pointers to heap structs)", what you want is an array of pointers. If you want to allocate that array of pointers automatically, your definition would look like:

obj *array[NUMBER];

My guess is that's not what you want though. Presumably, you want to allocate that array dynamically as well. That would look like this:

alt text

In this case, new_array and array will each need to be defined as a pointer to pointer to obj. You'd then allocate an array of pointers (i.e., pointers to as many objs as you want) and have each point point at an obj:

obj **new_array;

// allocate an array of pointers with space to point at more items:    
new_array = malloc(sizeof(obj *) * new_elements);

// copy the pointers to the current items to the new array:
for (i=0; i<current_elements; i++)
    new_array[i] = array[i];

The advantage of this is that when you do the copying, you only copy pointers, not the objects themselves. Especially with large objects, this can save a substantial amount of effort. The tradeoff is that using an element goes through two levels of indirection intead of one, so the reference may be slower (though rarely much slower, especially on a relatively high-performance processor).

As @rerun already pointed out, in either case you probably want to use realloc. In particular, this might be able to expand an allocation "in place", and avoid copying data as often. Of course, that's not guaranteed, but at least you're giving it a chance; if you malloc and copy every time, you eliminate even the possibility of that optimization.

Jerry Coffin
+1, Your answer has pictures. I guess I'll stop trying to answer with (nothing but) words now. :D
Jeff M
+1 for snazzy diagrams. "point point" is a strange thing to say, though, even if it is accurate :)
Merlyn Morgan-Graham
If I had enough reputation to upvote, I would :) Thanks, this has hit the nail on the head. was caught thinking pointers were pointers and you could drop any old address in them, forgetting the important point of the target type! so an obj** it is. I will also take a look at realloc(): it sounds like it would yeild optimisations farily frequently for small arrays of pointers. Thanks!
tehwalrus
@tehwalrus: `void*` is an "any-pointer" type. Pointers are all the same size, and you could cast them around, but it is better to document your intent via a good interface than try to simplify your code by doing (confusing) casts inside your implementation.
Merlyn Morgan-Graham
I think obj** is clearer in this case, don't you? :)
tehwalrus
A: 

in your code elements of the array are not pointers on the structure, they are structure objects. elements of the this array obj** array are pointers on the structure obj.

#define NUM_ELEM(x) (sizeof (x) / sizeof (*(x)))

void add_to_obj_array(obj* new_obj, obj** array)
{
  int number_of_elements = 0;
  if (array != NULL)
  {
    number_of_elements = NUM_ELEM(array);
  }

  // expand array with one more item
  array = (obj**)realloc(array, (number_of_elements + 1) * sizeof(new_obj));

  if (array == NULL )
  {
    /* memory request refused :( */
    return;
  }

  // Put new item at the last place on the array
  array[number_of_elements] = new_obj;
}

So here we used matrix (pointer on pointers of the obj structure). When we add new element we simply expand existing array for one place and on that place we put new structure object. There is no need to return value because we operate on the pointers of the objects, and all change is done on actual objects, not on their copies.

Mujo Osmanovic