views:

108

answers:

2

Hi,

In a separate library, we have a struct with:

typedef struct bsmat{
int m;
int *n;
double **d;
} bs;

where **d is a array of pointers to double arrays.

bs *new_bs(int n, double **d);

There are two use cases:

(a) The main application allocates multiple double matrices and calls the library to construct the structure.

b = new_bs(5, p)

(b) Alternatively, an object may be generated by the library as a result of a call to a function:

bs *add(bs *a, bs *b);

In the second case, the library owns **d and can free it when necessary. In the first case, the application has allocated the array and the library may read from/write to it. It is not clear to me as to who should free what and when??

Is it a problem to allow the main application to own a struct-member? What are the problems with this approach? What are the alternatives? We are trying to avoid copying large number of arrays back-and-forth.

thanks

TR

+3  A: 

In general, it's important that the library documents the 'contract' it makes with the applications that are using its service. The library writer documents all the allocations made by the library and what needs to be freed by the application. Like any contract, it's good when it is simple and consistent.

In this particular case, I suppose the library should also offer:

void free_bs(bs *b) 
{
     if (b->d) {
          /* free b->d here */
     }
     /* free rest of the structure */
}

which frees the structure. It makes sense to also free the d array here. The application has pointer to all the bs structures and is responsible for calling free_bs on them when no longer needed.

If, for example, you want to keep d for future usage, after you're done with the library operation, the contract is a bit more complex. The library could also provide:

double** bs_get_data(bs *b)
{
     double **d = b->d;
     b->d = NULL;
     return b->d;
}

which returns a pointer to d, and leaves the field empty so that the free routine knows to skip it.

The docs for the bs_get_data() have to explain that it can be called only once, and that the application takes the responsibility of freeing the array in this case.

UPDATE: In answer to your comment below: First of all, please note that I simplified the problem by assuming (at least) the following: the d is referenced either by a single bs structure or by the application. The application and the library "pass" a single reference to the array from one to the other. If you want the same d array in more bs structures, for example, than my approach is not enough.

The flag that you propose in the comment might help, but is not standard practice, FWIK. In this case, I would suggest to implement some simple reference counting. If d is the resource that needs to be shared around, make it an "object":

 typedef struct {
         double **d;
         int ref;
 } d_t;

Initialize ref with 1. In new_bs() and in all functions that receive a "copy" of d, increment the reference count. When the bs structure gets deleted, or when you no longer need d in your application, decrement it's reference count. If it gets to zero, free it. In a way, it is what the high level languages do for you, and is very efficient in keeping the resource management sane.

So no one is owning the array, but whoever needs it last, frees it.

Of course, this requires more work and complicates code, so try to put in balance. It's not needed for every structure you have, but only for those you need to keep multiple references to.

tsg
Thanks for the super quick response. Would it make sense to include a field "storageMode" (internal / external) which flags the ownership of **d? free_bs would then free (b->d), ONLY if owned internally. This would make the contract explicit, in that the caller is responsible for freeing **d? Are there other gotchas in using a shared object within a struct? It seems that OO people would frown upon such a practice!! Are there reasonable alternatives, not involving copying data??Thanks again, tr
I edited my answer in response, so I can write more :-). In short, you might want to consider reference counting.
tsg
+1  A: 

I think reference counting is overkill for this problem. But your interface needs to specify more precisely who owns *b and not just b->d. It's not at all clear to me how the library knows when it is "necessary" (as you put it) to free b->d.

I'd extend the interface as follows:

  1. Every pointer of type bs * is either externally managed or internally managed. Function new_bs returns an externally managed pointer; function add returns an internally managed one.

  2. Every function that returns a pointer of type bs * should say whether the management of the result is internal or external.

  3. You probably should provide functions

    void bs_free_ext(bs **);  // free *bs and set *bs = NULL (external mgmt)
    void bs_free_int(bs **);  // free *bs and set *bs = NULL (internal mgmt)
    

    bs_free_ext frees only *bs; bs_free_int frees both *bs and (*bs)->d.

  4. For safety's sake I would also add the field that records the storage management of each struct, and I would have assertions on any function that allocates or frees. (Reason: assertion checking is cheap compared to the cost of allocating or freeing.)

If you can run your app on Linux then memory checking with valgrind is a plus.

P.S. The fact that details of memory management leak into almost every interface is one of the costs we pay to get the benefits of programming in C.

Norman Ramsey
I used precisely 600 character in my previous comments!! Being new to C, nakedness of dynamic memory management and potentially unhygenic sharing of memory location between the application and the library is scary. Are there any other things I need to worry about during the life of bs and b->d about the two parties inadervently stepping on each other's toes? I expect the App to update B->d and the library to use BS in computations- at the request of the App. But, I am sure there are unanticipated ways in which non-consensual data exchanges may happen! Any safety mechanisms to prevent it?