label inside for-loop

C

Capstar

Hi NG,

I'm trying to allocate some resources, in this example a structure, with
containing a buffer. When a allocation failes, all previous allocations
need to be freed. When I try to compile it I get the next warning:

resource-loop.c:38: warning: deprecated use of label at end of compound
statement

Is there a better way of doing this?
Or is this guaranteed to work anyway?

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

struct foo
{
char *bar;
};

int main(void)
{
int i;
struct foo* array[6];

for(i = 0; i < 6; ++i)
{
if(!(array = malloc(sizeof *array)))
{
fprintf(stderr, "Unable to allocate foo%d\n", i);
goto err_malloc_foo;
}

if(!(array->bar = malloc(1024)))
{
fprintf(stderr, "Unable to allocate bar%d\n", i);
goto err_malloc_bar;
}
}

return EXIT_SUCCESS;

for(; i >= 0; --i)
{
free(array->bar);
err_malloc_bar:

free(array);
err_malloc_foo:
}

return EXIT_FAILURE;
}


Mark
 
C

Capstar

Capstar said:
Hi NG,

I'm trying to allocate some resources, in this example a structure, with
containing a buffer. When a allocation failes, all previous allocations
need to be freed. When I try to compile it I get the next warning:

resource-loop.c:38: warning: deprecated use of label at end of compound
statement

Is there a better way of doing this?
Or is this guaranteed to work anyway?

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

struct foo
{
char *bar;
};

int main(void)
{
int i;
struct foo* array[6];

for(i = 0; i < 6; ++i)
{
if(!(array = malloc(sizeof *array)))
{
fprintf(stderr, "Unable to allocate foo%d\n", i);
goto err_malloc_foo;
}

if(!(array->bar = malloc(1024)))
{
fprintf(stderr, "Unable to allocate bar%d\n", i);
goto err_malloc_bar;
}
}

return EXIT_SUCCESS;

for(; i >= 0; --i)
{
free(array->bar);
err_malloc_bar:

free(array);
err_malloc_foo:
}

return EXIT_FAILURE;
}


Mark
Click to expand...


Ok, I just thought of another way of doing the last piece:

return EXIT_SUCCESS;

while(i >= 0)
{
free(array->bar);
err_malloc_bar:

free(array);
err_malloc_foo:

--i;
}

return EXIT_FAILURE;

The warning is gone now, which makes sence because the label is not at
the end of a compound statement anymore. But why does this make any
difference? It seems to me that functionally it didn't change at all.

Or am I missing something here?

Mark
 
H

Harti Brandt

On Tue, 8 Jun 2004, Capstar wrote:

C>Capstar wrote:
C>> Hi NG,
C>>
C>> I'm trying to allocate some resources, in this example a structure, with
C>> containing a buffer. When a allocation failes, all previous allocations need
C>> to be freed. When I try to compile it I get the next warning:
C>>
C>> resource-loop.c:38: warning: deprecated use of label at end of compound
C>> statement
C>>
C>> Is there a better way of doing this?
C>> Or is this guaranteed to work anyway?
C>>
C>> #include <stdlib.h>
C>> #include <stdio.h>
C>>
C>> struct foo
C>> {
C>> char *bar;
C>> };
C>>
C>> int main(void)
C>> {
C>> int i;
C>> struct foo* array[6];
C>>
C>> for(i = 0; i < 6; ++i)
C>> {
C>> if(!(array = malloc(sizeof *array)))
C>> {
C>> fprintf(stderr, "Unable to allocate foo%d\n", i);
C>> goto err_malloc_foo;
C>> }
C>>
C>> if(!(array->bar = malloc(1024)))
C>> {
C>> fprintf(stderr, "Unable to allocate bar%d\n", i);
C>> goto err_malloc_bar;
C>> }
C>> }
C>>
C>> return EXIT_SUCCESS;
C>>
C>> for(; i >= 0; --i)
C>> {
C>> free(array->bar);
C>> err_malloc_bar:
C>>
C>> free(array);
C>> err_malloc_foo:
C>> }
C>>
C>> return EXIT_FAILURE;
C>> }
C>>
C>>
C>> Mark
C>>
C>
C>Ok, I just thought of another way of doing the last piece:
C>
C> return EXIT_SUCCESS;
C>
C> while(i >= 0)
C> {
C> free(array->bar);
C>err_malloc_bar:
C>
C> free(array);
C>err_malloc_foo:
C>
C> --i;
C> }
C>
C> return EXIT_FAILURE;
C>
C>The warning is gone now, which makes sence because the label is not at the end
C>of a compound statement anymore. But why does this make any difference? It
C>seems to me that functionally it didn't change at all.
C>
C>Or am I missing something here?

A label always stands before a statement. If you need a label at the end
of a compound just put a null-statement after the label:

foo: ;

harti
 
B

boa

Maybe, how about this version? It has no goto's and has fewer calls to
malloc, hopefully leading to faster code and less fragmented memory.

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

struct foo {
char *bar;
};

#define NELEM 6

int main(void)
{
int i;
struct foo* array;

if( (array = malloc(sizeof *array * NELEM)) == NULL)
return EXIT_FAILURE;

for(i = 0; i < NELEM; i++) {
if( (array.bar = malloc(1024)) == NULL) {
while(--i >= 0)
free(array.bar);
free(array);
return EXIT_FAILURE;
}
}

return EXIT_SUCCESS;
}


HTH,
boa
[snip]
 
C

Capstar

boa said:
Maybe, how about this version? It has no goto's and has fewer calls to
malloc, hopefully leading to faster code and less fragmented memory.

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

struct foo {
char *bar;
};

#define NELEM 6

int main(void)
{
int i;
struct foo* array;

if( (array = malloc(sizeof *array * NELEM)) == NULL)
return EXIT_FAILURE;

for(i = 0; i < NELEM; i++) {
if( (array.bar = malloc(1024)) == NULL) {
while(--i >= 0)
free(array.bar);
free(array);
return EXIT_FAILURE;
}
}

return EXIT_SUCCESS;
}
Click to expand...


This is about how I would normally do this, but this was just some
example code. The actual code is part of the initialisation code for a
device driver. So there are lots of calls, which can fail. And if one
fails all previous calls that allocate or register stuff need to be
undone. That can offcourse be done without any goto's but, but That
would mean lots of nested if-else statements, and make my code complete
unreadable. This loop thing is ment for requesting some pci space and
mapping it to virtual memory space. In my previous version I just copied
the code 6 times. So this was actually an effort to clean things up a bit.

But thanks for your input anyway.

Mark
 
C

Capstar

Harti said:
On Tue, 8 Jun 2004, Capstar wrote:

C>Capstar wrote:
C>> Hi NG,
C>>
C>> I'm trying to allocate some resources, in this example a structure, with
C>> containing a buffer. When a allocation failes, all previous allocations need
C>> to be freed. When I try to compile it I get the next warning:
C>>
C>> resource-loop.c:38: warning: deprecated use of label at end of compound
C>> statement
C>>
C>> Is there a better way of doing this?
C>> Or is this guaranteed to work anyway?
C>>
C>> #include <stdlib.h>
C>> #include <stdio.h>
C>>
C>> struct foo
C>> {
C>> char *bar;
C>> };
C>>
C>> int main(void)
C>> {
C>> int i;
C>> struct foo* array[6];
C>>
C>> for(i = 0; i < 6; ++i)
C>> {
C>> if(!(array = malloc(sizeof *array)))
C>> {
C>> fprintf(stderr, "Unable to allocate foo%d\n", i);
C>> goto err_malloc_foo;
C>> }
C>>
C>> if(!(array->bar = malloc(1024)))
C>> {
C>> fprintf(stderr, "Unable to allocate bar%d\n", i);
C>> goto err_malloc_bar;
C>> }
C>> }
C>>
C>> return EXIT_SUCCESS;
C>>
C>> for(; i >= 0; --i)
C>> {
C>> free(array->bar);
C>> err_malloc_bar:
C>>
C>> free(array);
C>> err_malloc_foo:
C>> }
C>>
C>> return EXIT_FAILURE;
C>> }
C>>
C>>
C>> Mark
C>>
C>
C>Ok, I just thought of another way of doing the last piece:
C>
C> return EXIT_SUCCESS;
C>
C> while(i >= 0)
C> {
C> free(array->bar);
C>err_malloc_bar:
C>
C> free(array);
C>err_malloc_foo:
C>
C> --i;
C> }
C>
C> return EXIT_FAILURE;
C>
C>The warning is gone now, which makes sence because the label is not at the end
C>of a compound statement anymore. But why does this make any difference? It
C>seems to me that functionally it didn't change at all.
C>
C>Or am I missing something here?

A label always stands before a statement. If you need a label at the end
of a compound just put a null-statement after the label:

foo: ;
Click to expand...


Thanks, that did the trick.

After opening a book after reading this, I found out that a goto points
to a 'labelled statement', and not to just a 'label' as I used to think.

Never to old to learn
 
D

Darrell Grainger

Hi NG,

I'm trying to allocate some resources, in this example a structure, with
containing a buffer. When a allocation failes, all previous allocations
need to be freed. When I try to compile it I get the next warning:

resource-loop.c:38: warning: deprecated use of label at end of compound
statement

Is there a better way of doing this?
Click to expand...

Allocate one large block of memory and initialize the pointers so they
point into that one large block. One call to malloc, one call to free.
Or is this guaranteed to work anyway?

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

struct foo
{
char *bar;
};

int main(void)
{
int i;
struct foo* array[6];

for(i = 0; i < 6; ++i)
{
if(!(array = malloc(sizeof *array)))
{
fprintf(stderr, "Unable to allocate foo%d\n", i);
goto err_malloc_foo;
}

if(!(array->bar = malloc(1024)))
{
fprintf(stderr, "Unable to allocate bar%d\n", i);
goto err_malloc_bar;
}
}

return EXIT_SUCCESS;

for(; i >= 0; --i)
{
free(array->bar);
err_malloc_bar:

free(array);
err_malloc_foo:
}

return EXIT_FAILURE;
}


Mark
Click to expand...
 
S

SM Ryan

# resource-loop.c:38: warning: deprecated use of label at end of compound
# statement

Use an empty statement.

# free(array);
# err_malloc_foo:
# }

free(array);
err_malloc_foo:
;
}
 
C

Chris Torek

[ "goto label; ... { ... label: }" produces the complaint ]
resource-loop.c:38: warning: deprecated use of label at end of compound
statement
Click to expand...

As others have noted, the immediate fix is to use a null statement.

It is worth pointing out that what is "deprecated" here is actually
a GNUC extension -- the syntax above has always been invalid in
ANSI C. The GCC folks decided to allow it, and have now decided
to stop allowing it.

For those who want to find one, there is a moral in here about
depending on compiler-specific extensions. :)
 
M

Mark F. Haigh

Capstar said:
Hi NG,

I'm trying to allocate some resources, in this example a structure, with
containing a buffer. When a allocation failes, all previous allocations
need to be freed. When I try to compile it I get the next warning:

resource-loop.c:38: warning: deprecated use of label at end of compound
statement
Click to expand...

Look at the following section in the C99 standard:

A.2.3 Statements

(6.8.1) labeled-statement:
identifier : statement
case constant-expression : statement
default : statement

You'll notice that a label does not stand alone; rather, it's labeling
a statement. What's prompting the warning from the compiler is that
you're labeling the closing bracket.

Instead, simply label an empty statement like:

for(; i >= 0; --i)
{
free(array->bar);
err_malloc_bar:

free(array);
err_malloc_foo:
Click to expand...

; /* empty statement */
}

return EXIT_FAILURE;
}
Click to expand...


Mark F. Haigh
(e-mail address removed)
 
J

josh

Capstar said:
Hi NG,

I'm trying to allocate some resources, in this example a structure, with
containing a buffer. When a allocation failes, all previous allocations
need to be freed. When I try to compile it I get the next warning:

resource-loop.c:38: warning: deprecated use of label at end of compound
statement

Is there a better way of doing this?
Or is this guaranteed to work anyway?
Click to expand...

Since free(NULL) does nothing, you could do:

[...]
for(; i >= 0; --i)
{
Click to expand...
err_malloc_bar:
free(array->bar);

err_malloc_foo:
free(array);
}

return EXIT_FAILURE;
}
Click to expand...

Alternatively:

struct foo
{
char *bar, *baz;
};

int main(void)
{
int i;
struct foo* array[6];
for (i=0; i<6; ++i)
{
if (!(array = malloc(sizeof array)))
break;
if (!(array.bar = malloc(1024)))
break;
if (!(array.baz = malloc(256)))
break;
}
if (i == 6)
return EXIT_SUCCESS;

if (!array) --i;
for (; i>=0; free(array[i--]))
{
/* check in the same order as allocation */
/* when we hit a NULL, the rest are unallocated */
if (!array.bar) continue;
free(array.bar);
if (!array.baz) continue;
free(array.baz);
}
return EXIT_FAILURE;
}

Or you could have a second variable that you set to 0 at the start of
the alloc loop and increment at each step. On failure, do a nasty
duff-esque switch/for:

int main(void)
{
int i,fail_step;
struct foo* array[6];
for (i=0; i<6; ++i)
{
fail_step = 0;
if (!(array = malloc(sizeof array)))
break;
++fail_step;
if (!(array.bar = malloc(1024)))
break;
++fail_step;
if (!(array.baz = malloc(256)))
break;
}
if (i == 6)
return EXIT_SUCCESS;

switch (fail_step)
{
for (; i>=0; --i)
{
/* needs to be in reverse of alloc order */
case 2: free(array.baz]);
case 1: free(array.bar]);
case 0: free(array);
}
}
return EXIT_FAILURE;
}

but I dunno if that's really any better than goto. (it'd really suck to
add a step in the middle...)

-josh
 

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

Similar Threads

Linux: using "clone3" and "waitid" 0
struct inside struct 5
Adding adressing of IPv6 to program 1
expression-statement question 4
Queue - can not delete last element 6
Request for source code review of simple Ising model 88
Problem with "restrict" 25
void pointers and scanf/printf 0

Members online

Total: 24 (members: 2, guests: 22)
Robots: 378

Forum statistics

Threads
473,769
Messages
2,569,582
Members
45,070
Latest member
BiogenixGummies

Latest Threads

Top