Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #10339 > unrolled thread

Handing initialization failures

Back to article view | Back to comp.lang.c

Contents

  Handing initialization failures Максим Фомин <maxim@maxim-fomin.ru> - 2011-09-16 11:42 -0700
    Re: Handing initialization failures James Kuyper <jameskuyper@verizon.net> - 2011-09-16 15:20 -0400
      Re: Handing initialization failures Максим Фомин <maxim@maxim-fomin.ru> - 2011-09-16 21:50 -0700
    Re: Handing initialization failures Ben Bacarisse <ben.usenet@bsb.me.uk> - 2011-09-16 20:26 +0100
    Re: Handing initialization failures ImpalerCore <jadill33@gmail.com> - 2011-09-16 12:29 -0700
    Re: Handing initialization failures jacob navia <jacob@spamsink.net> - 2011-09-16 21:45 +0200
      Re: Handing initialization failures Ian Collins <ian-news@hotmail.com> - 2011-09-17 08:15 +1200
    Re: Handing initialization failures "christian.bau" <christian.bau@cbau.wanadoo.co.uk> - 2011-09-16 13:41 -0700
    Re: Handing initialization failures Malcolm McLean <malcolm.mclean5@btinternet.com> - 2011-09-16 13:47 -0700
    Re: Handing initialization failures Paul N <gw7rib@aol.com> - 2011-09-17 04:04 -0700
      Re: Handing initialization failures James Kuyper <jameskuyper@verizon.net> - 2011-09-17 15:16 -0400
    Re: Handing initialization failures Nobody <nobody@nowhere.com> - 2011-09-17 19:16 +0100
    Re: Handing initialization failures Lauri Alanko <la@iki.fi> - 2011-09-21 19:09 +0000
      Re: Handing initialization failures Gene <gene.ressler@gmail.com> - 2011-09-22 06:32 -0700

#10339 — Handing initialization failures

Hello.

I have a question regarding following problem. Let there is

struct Container{
void *buffer;
resource1 *res1;
resource2 *res2;
...
resource_n *res_n;
};

pointer to which is return by function like container_alloc().
Initialization of each member may fail and if last fails, and I need
to free() all other initialized resources (otherwise there would be
memory leakage). The situation become worse when any random "sub-
resource" initialization may fail.

What is best way to deal whis this? There is obvious solution to free
all previous allocated resources within "if" test block of each
element, however I am looking for a better solution.

[toc] | [next] | [standalone]

#10342

On 09/16/2011 02:42 PM, Максим Фомин wrote:
> Hello.
> 
> I have a question regarding following problem. Let there is
> 
> struct Container{
> void *buffer;
> resource1 *res1;
> resource2 *res2;
> ...
> resource_n *res_n;
> };
> 
> pointer to which is return by function like container_alloc().
> Initialization of each member may fail and if last fails, and I need
> to free() all other initialized resources (otherwise there would be
> memory leakage). The situation become worse when any random "sub-
> resource" initialization may fail.
> 
> What is best way to deal whis this? There is obvious solution to free
> all previous allocated resources within "if" test block of each
> element, however I am looking for a better solution.

Set all of the pointers to null at start.
If any allocation fails, free ALL of the pointers.
Situations like this are part of the reason why the standard allows null
pointers to be passed to free().

[toc] | [prev] | [next] | [standalone]

#10377

On Sep 16, 11:20 pm, James Kuyper <jameskuy...@verizon.net> wrote:
>
> > What is best way to deal whis this? There is obvious solution to free
> > all previous allocated resources within "if" test block of each
> > element, however I am looking for a better solution.
>
> Set all of the pointers to null at start.
> If any allocation fails, free ALL of the pointers.
> Situations like this are part of the reason why the standard allows null
> pointers to be passed to free().

Actually that resources are not dynamic memory, they are allocated in
the way struct Container does, but I can make allocation functions
behave like free(NULL) does.

[toc] | [prev] | [next] | [standalone]

#10343

Максим Фомин <maxim@maxim-fomin.ru> writes:

> I have a question regarding following problem. Let there is
>
> struct Container{
> void *buffer;
> resource1 *res1;
> resource2 *res2;
> ...
> resource_n *res_n;
> };
>
> pointer to which is return by function like container_alloc().
> Initialization of each member may fail and if last fails, and I need
> to free() all other initialized resources (otherwise there would be
> memory leakage). The situation become worse when any random "sub-
> resource" initialization may fail.
>
> What is best way to deal whis this? There is obvious solution to free
> all previous allocated resources within "if" test block of each
> element, however I am looking for a better solution.

One solution is to initialise the struct first so that all the pointers
are null.  Then you can safely free everything because free(0) is a
no-op.  One way to do this is to keep a

  static struct Container zero;

so that you can do

  struct Container *sp = malloc(sizeof *sp); 
  if (sp != NULL) {
      *sp = zero;
      if ((sp->res1 = malloc(sizeof *sp->res1)) == NULL)
          goto clean_up;
      /* etc. */
  }

If the 'res' members were in an array you would not need to do this --
the code is often simpler with arrays:

  for (i = 0; i < n; i++)
      if ((sp->res[i] = malloc(sizeof *sp->res[i])) == NULL) {
          for (j = 0; j < i; j++)
              free(sp->res[j]);
          break;
      }

-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#10344

On Sep 16, 2:42 pm, Максим Фомин <ma...@maxim-fomin.ru> wrote:
> Hello.
>
> I have a question regarding following problem. Let there is
>
> struct Container{
> void *buffer;
> resource1 *res1;
> resource2 *res2;
> ...
> resource_n *res_n;
>
> };
>
> pointer to which is return by function like container_alloc().
> Initialization of each member may fail and if last fails, and I need
> to free() all other initialized resources (otherwise there would be
> memory leakage). The situation become worse when any random "sub-
> resource" initialization may fail.
>
> What is best way to deal whis this? There is obvious solution to free
> all previous allocated resources within "if" test block of each
> element, however I am looking for a better solution.

Here is a possible way to deal with it.

\code untested
struct Container* container_alloc( size_of_buffer, r1, ..., rn )
{
  struct Container* new_container = NULL;
  bool alloc_failed = false;

  new_container = malloc( sizeof (struct Container) );
  if ( new_container )
  {
     new_container->buffer = !alloc_failed ?
malloc( size_of_buffer ) : NULL;
     alloc_failed = new_container->buffer == NULL;

     new_container->res1 = !alloc_failed ?
malloc( size_of_resource1 ) : NULL;
     alloc_failed = new_container->res1 == NULL;

     new_container->res2 = !alloc_failed ?
malloc( size_of_resource2 ) : NULL;
     alloc_failed = new_container->res2 == NULL;

     ...

     new_container->resN = !alloc_failed ?
malloc( size_of_resourceN ) : NULL;
     alloc_failed = new_container->resN == NULL;

     if ( alloc_failed )
     {
       free( new_container->buffer );
       free( new_container->res1 );
       free( new_container->res2 );
       ...
       free( new_container->resN );
       free( new_container );
       new_container = NULL;
     }
  }

  return new_container;
}
\endcode

The 'free' function allows freeing NULL pointers, so if an allocation
fault occurs, it should clean any partial resources that's been
allocated.  Be sure to test it, as it's just off the top of my head.

Best regards,
John D.

[toc] | [prev] | [next] | [standalone]

#10346

Le 16/09/11 20:42, Максим Фомин a écrit :
> Hello.
>
> I have a question regarding following problem. Let there is
>
> struct Container{
> void *buffer;
> resource1 *res1;
> resource2 *res2;
> ...
> resource_n *res_n;
> };
>
> pointer to which is return by function like container_alloc().
> Initialization of each member may fail and if last fails, and I need
> to free() all other initialized resources (otherwise there would be
> memory leakage). The situation become worse when any random "sub-
> resource" initialization may fail.
>
> What is best way to deal whis this? There is obvious solution to free
> all previous allocated resources within "if" test block of each
> element, however I am looking for a better solution.

Besides the other answers you got consider using a garbage collector.
All these problems go away with that solution. (Others may appear).

Under windows you can use the gc dll with any compiler, as in linux.

[toc] | [prev] | [next] | [standalone]

#10347

On 09/17/11 07:45 AM, jacob navia wrote:
> Le 16/09/11 20:42, Максим Фомин a écrit :
>> Hello.
>>
>> I have a question regarding following problem. Let there is
>>
>> struct Container{
>> void *buffer;
>> resource1 *res1;
>> resource2 *res2;
>> ...
>> resource_n *res_n;
>> };
>>
>> pointer to which is return by function like container_alloc().
>> Initialization of each member may fail and if last fails, and I need
>> to free() all other initialized resources (otherwise there would be
>> memory leakage). The situation become worse when any random "sub-
>> resource" initialization may fail.
>>
>> What is best way to deal whis this? There is obvious solution to free
>> all previous allocated resources within "if" test block of each
>> element, however I am looking for a better solution.
>
> Besides the other answers you got consider using a garbage collector.
> All these problems go away with that solution. (Others may appear).

Only if the resource in question is memory.

-- 
Ian Collins

[toc] | [prev] | [next] | [standalone]

#10348

> pointer to which is return by function like container_alloc().
> Initialization of each member may fail and if last fails, and I need
> to free() all other initialized resources (otherwise there would be
> memory leakage). The situation become worse when any random "sub-
> resource" initialization may fail.

You have a function container_alloc (), and you should have another
function container_free (). container_alloc () returns a properly
initialised container or NULL. The first thing it does is malloc () a
container, and if that succeeds then it sets it up so that
container_free () will work properly (for example by setting pointers
to NULL). Then you allocate the pointers, check that they are all
fine, and if not you pass the container to conter_free (). Random
failures for sub resources is no problem; they would return NULL on
failure which you check for. It's easier if there is no need to stop
after a failure:

So:
   container* p = malloc (sizeof (container));
   if (p != NULL) {
        p->res1 = resource_alloc ();
        p->res2 = resource_alloc ();
        p->res3 = resource_alloc ();
        if (p->res1 == NULL || p->res2 == NULL || p->res3 == NULL) {
            container_free (NULL); p = NULL;
        }
    }
    return p;

[toc] | [prev] | [next] | [standalone]

#10350

On Sep 16, 9:42 pm, Максим Фомин <ma...@maxim-fomin.ru> wrote:
> Hello.
>
> I have a question regarding following problem. Let there is
>
> struct Container{
> void *buffer;
> resource1 *res1;
> resource2 *res2;
> ...
> resource_n *res_n;
>
> };
>
> pointer to which is return by function like container_alloc().
> Initialization of each member may fail and if last fails, and I need
> to free() all other initialized resources (otherwise there would be
> memory leakage). The situation become worse when any random "sub-
> resource" initialization may fail.
>
> What is best way to deal whis this? There is obvious solution to free
> all previous allocated resources within "if" test block of each
> element, however I am looking for a better solution.
>
I just write

res1 = allocateresource1();
if(res1 == NULL)
  goto outofmemory;

...
outofmemory:
  cleanupeverything;
  return NULL;

You sometimes have to be careful to set all the pointers to NULL, so
that they can be either passed to free harmlessly or detected as
unallocated and not freed.
--
Basic algorithms, in C. Full JPEG codec.
http://www.malcolmmclean.site11.com/www

[toc] | [prev] | [next] | [standalone]

#10393

On Sep 16, 7:42 pm, Максим Фомин <ma...@maxim-fomin.ru> wrote:
> Hello.
>
> I have a question regarding following problem. Let there is
>
> struct Container{
> void *buffer;
> resource1 *res1;
> resource2 *res2;
> ...
> resource_n *res_n;
>
> };
>
> pointer to which is return by function like container_alloc().
> Initialization of each member may fail and if last fails, and I need
> to free() all other initialized resources (otherwise there would be
> memory leakage). The situation become worse when any random "sub-
> resource" initialization may fail.
>
> What is best way to deal whis this? There is obvious solution to free
> all previous allocated resources within "if" test block of each
> element, however I am looking for a better solution.

One possible approach - no-one else has mentioned it, perhaps that
suggests it's no good - would be to have nested ifs. For instance,
something like:

struct Container *container_alloc(void) {
// allocate container c itself
c -> res1 = whatever
if (worked) {
  c -> res2 = whatever
  if (worked) {
    return c; }
  free res2 }
free res1
return NULL;
}

[toc] | [prev] | [next] | [standalone]

#10407

On 09/17/2011 07:04 AM, Paul N wrote:
> On Sep 16, 7:42 pm, Максим Фомин <ma...@maxim-fomin.ru> wrote:
>> Hello.
>>
>> I have a question regarding following problem. Let there is
>>
>> struct Container{
>> void *buffer;
>> resource1 *res1;
>> resource2 *res2;
>> ...
>> resource_n *res_n;
>>
>> };
>>
>> pointer to which is return by function like container_alloc().
>> Initialization of each member may fail and if last fails, and I need
>> to free() all other initialized resources (otherwise there would be
>> memory leakage). The situation become worse when any random "sub-
>> resource" initialization may fail.
>>
>> What is best way to deal whis this? There is obvious solution to free
>> all previous allocated resources within "if" test block of each
>> element, however I am looking for a better solution.
> 
> One possible approach - no-one else has mentioned it, perhaps that
> suggests it's no good - would be to have nested ifs. For instance,
> something like:
> 
> struct Container *container_alloc(void) {
> // allocate container c itself
> c -> res1 = whatever
> if (worked) {
>   c -> res2 = whatever
>   if (worked) {
>     return c; }
>   free res2 }
> free res1
> return NULL;
> }

I believe that this is basically the "obvious solution" that he was
referring to. The problem with that solution is that it gets very
unwieldy when the list of resources to be allocated gets long.
-- 
James Kuyper

[toc] | [prev] | [next] | [standalone]

#10403

On Fri, 16 Sep 2011 11:42:36 -0700, Максим Фомин wrote:

> What is best way to deal whis this?

C++ and RAII ;)

> There is obvious solution to free all
> previous allocated resources within "if" test block of each element,
> however I am looking for a better solution.

One solution not mentioned so far is:

	obj = obj_alloc();
	if (!obj)
	   goto fail0;
	obj->res1 = res1_alloc();
	if (!obj->res1)
	   goto fail1;
	obj->res2 = res2_alloc();
	if (!obj->res2)
	   goto fail2;
	...
	return obj;	/* success */
	...
	fail3:
	    res2_free(obj->res2);
	fail2:
	    res1_free(obj->res1);
	fail1:
	    obj_free(obj);
	fail0:
	    return NULL;

Personally, I'd just go with having all of the _free() functions ignore
NULL pointers, so that you can legitimately call the top-level obj_free()
function on a partially-constructed object.

[toc] | [prev] | [next] | [standalone]

#10669

In article <013d83d0-af71-443a-b9a8-f40b0f28f061@h9g2000yqi.googlegroups.com>,
а аАаКб аИаМ аЄаОаМаИаН  <maxim@maxim-fomin.ru> wrote:
> struct Container{
> void *buffer;
> resource1 *res1;
> resource2 *res2;
> ...
> resource_n *res_n;
> };
> 
> pointer to which is return by function like container_alloc().
> Initialization of each member may fail and if last fails, and I need
> to free() all other initialized resources (otherwise there would be
> memory leakage). The situation become worse when any random "sub-
> resource" initialization may fail.
> 
> What is best way to deal whis this?

I have found the following approach relatively neat, and I think it
demonstrates an acceptable use of goto:

Container* container_new(void)
{
	Container* c = container_alloc();
	if (!c) goto fail_c;

	c->res1 = resoure1_new();
	if (!c->res1) goto fail_res1;

	c->res2 = resource2_new();
	if (!c->res2) goto fail_res2;

	...

	c->res_n = resource_n_new();
	if (!c->res_n) goto fail_res_n;

	return c;

fail_res_n:
	...
	resource2_destroy(c->res2);
fail_res2:
	resource1_destroy(c->res1);
fail_res1:
	container_dealloc(c);
fail_c:
	return NULL;
}

The good thing with this approach is that the code is quite clean and
all error handling is explicit. On the other hand, the error handling
may then duplicate some code with a container_destroy function that
would free a Container and its contents once they were all
successfully allocated. I think this is a reasonable tradeoff, though.


Lauri

[toc] | [prev] | [next] | [standalone]

#10702

On Sep 21, 3:09 pm, Lauri Alanko <l...@iki.fi> wrote:
> In article <013d83d0-af71-443a-b9a8-f40b0f28f...@h9g2000yqi.googlegroups.com>,
> Ð Ð°ÐºÑ Ð¸Ð¼ Фомин  <ma...@maxim-fomin.ru> wrote:
>
> > struct Container{
> > void *buffer;
> > resource1 *res1;
> > resource2 *res2;
> > ...
> > resource_n *res_n;
> > };
>
> > pointer to which is return by function like container_alloc().
> > Initialization of each member may fail and if last fails, and I need
> > to free() all other initialized resources (otherwise there would be
> > memory leakage). The situation become worse when any random "sub-
> > resource" initialization may fail.
>
> > What is best way to deal whis this?
>
> I have found the following approach relatively neat, and I think it
> demonstrates an acceptable use of goto:
>
> Container* container_new(void)
> {
>         Container* c = container_alloc();
>         if (!c) goto fail_c;
>
>         c->res1 = resoure1_new();
>         if (!c->res1) goto fail_res1;
>
>         c->res2 = resource2_new();
>         if (!c->res2) goto fail_res2;
>
>         ...
>
>         c->res_n = resource_n_new();
>         if (!c->res_n) goto fail_res_n;
>
>         return c;
>
> fail_res_n:
>         ...
>         resource2_destroy(c->res2);
> fail_res2:
>         resource1_destroy(c->res1);
> fail_res1:
>         container_dealloc(c);
> fail_c:
>         return NULL;
>
> }
>
> The good thing with this approach is that the code is quite clean and
> all error handling is explicit. On the other hand, the error handling
> may then duplicate some code with a container_destroy function that
> would free a Container and its contents once they were all
> successfully allocated. I think this is a reasonable tradeoff, though.
>
> Lauri

I've used this idea and it works fine in a straight-line procedure,
but doesn't hold up if allocation ends up being conditional.  At that
point it just falls apart.

Over years I've fallen into this strategy:

{
  Declarations
  Initializers:  Put each data structure in a known state where no
resources have been allocated.

  Setup:  Assign resources data structures. If any part of an
allocation fails, return it to the initialized state.  Allocators can
(and often should) be called lazily: just before the respective
structure is used.

  Uses:  Use the data structures.  The uses can employ an
is_allocated() predicate for each data structure to decide if they can
run successfully.  Any kind of abort does a   goto cleanup;

cleanup:
  Clearers: Return each data structure to its initialized state.
}

The secret to happiness is the invariants: (1) init_ is always called
exactly once just after declaration.  (2) Setup is idempotent.  You
can call it repeatedly, potentially with different parameters, and
it's no harm no foul.  (3) Cleanup is also idempotent. If a previous
setup_ succeeded, it releases held resources and returns all to the
init_ state. Additional calls do nothing.

If you were implementing a vector data type in this scheme, it would
be:

typedef struct float_vector_s {
  unsigned n_elts;
  float *elts;
} FLOAT_VECTOR;

void init_float_vector(FLOAT_VECTOR *v)
{
  v->n_elts = ~0u;  // "initialized not allocated"
  v->elts = NULL;
}

void clear_float_vector(FLOAT_VECTOR *v)
{
  free(v->elts);
  // clear_ (almost) always ends with init_
  init_float_vector(v);
}

void setup_float_vector(FLOAT_VECTOR *v, unsigned n_elts)
{
  // Setup (almost) always clears first.
  clear_float_vector(v);
  if (n_elts > 0) {
    v->elts = malloc(n_elts * sizeof *v->elts);
    if (v->elts) v->n_elts = n_elts;
  }
  else v->n_elts = 0; // 0-vectors are okay...
}

int is_float_vector_setup_ok(FLOAT_VECTOR *v)
{
  return v->n_elts != ~0u;
}

Now in use it looks like this:

FLOAT_VECTOR v[1]; // 1-element array saves &'s.
init_float_vector(v); // just once!

// stuff

if (I need a vector) {
  setup_float_vector(v, 100);

  //  We can test success.
  if (is_float_vector_setup_ok(v)) {

    // oops need more space. No problem.
    setup_float_vector(v, 200);

    // abort if space couldn't be found
    if (!is_float_vector_setup_ok(v)) go cleanup;
  }

  // something might go wrong.  No problem...
  if (bad stuff happens) goto cleanup;

  // If you need the space back here, just clear it
  clear_float_vector(v);

  // something else might go wrong.  It's no problem
  // that v is already clear.
  if (bad stuff happens) goto cleanup;
}

cleanup:
  clear_float_vector(v); // Idempotent, so an extra call doesn't
matter.
  ... and all the other clear_ routines are called here.


Note that this just generalizes the free(NULL) mechanism James Kuyper
mentioned.

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.c

csiph-web