Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Strange error when reallocating memory

83 views
Skip to first unread message

francesco

unread,
Dec 22, 2012, 1:09:50 PM12/22/12
to
I get this strange error message


subsets.exe: malloc.c:2451: sYSMALLOc: Assertion `(old_top == (((mbinptr)
(((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct
malloc_chunk, fd)))) && old_size == 0) || ((unsigned long) (old_size) >=
(unsigned long)((((__builtin_offsetof (struct malloc_chunk, fd_nextsize))
+((2 * (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) - 1))) &&
((old_top)->size & 0x1) && ((unsigned long)old_end & pagemask) == 0)'
failed.

The program works correcly for a certain numbers of data, say n=7, when I
try it for n>7, I get this error message.

I need to dinamically allocate memory for 2^n structs. It workd properly
until I try the program for n=8 or greater, then it fails.

Francesco

Ian Collins

unread,
Dec 22, 2012, 1:33:51 PM12/22/12
to
You have an error at line 42.

--
Ian Collins

francesco

unread,
Dec 22, 2012, 2:21:12 PM12/22/12
to
>
> You have an error at line 42.

Sorry, this is he code

#include<stdlib.h>
#include<stdio.h>
typedef struct {
int *ptrSet;
int elementi;
}Set;
int pot(int base,int esponente);
void stamparesti( int *resti, int n);
int main()
{
Set *ptrPDS=NULL;
int *ptr2Set=NULL;
int *resti=NULL;
int *set=NULL;
int n,m;
int i=0,j=0;
int N,Q=0;
int card=0;
printf("Inserisci il numero di elementi di S\n");
scanf("%d",&n);
m=pot(2,n);
printf("L'insieme dei sottoinsiemi di un insieme S ha %d sottoinsiemi
\n",m);
ptrPDS=(Set *)malloc(sizeof(Set)*m);
if(ptrPDS==NULL)
{
printf("Impossibile allocare spazio per l'insieme PDS: memoria
insufficiente\n");
free(ptrPDS);
exit(1);
}
set=malloc(sizeof(int)*n);
// Verifica se esiste abbastanza memoria per l'allocazione dell'insieme
if(set==NULL)
{
printf("Impossibile allocare spazio per l'insieme S: memoria
insufficiente\n");
free(set);
exit(1);
}

printf("Iserire gli elementi di S\n");
for(i=0;i<n;i++)
{
printf("\nInserire elemento %d",i);scanf("%d",set+i);
}
for(i=0;i<m;i++)
{
printf("i=%d\n",i);
resti=malloc(sizeof(int)*n);
if(resti==NULL)
{
printf("Impossibile allocare spazio per l'insieme S:
memoria insufficiente\n");
free(resti);
exit(1);
}
N=i;
//Conversione in binario del numero i
for(j=0;j<n;j++)
{
Q=N/2;
*(resti+j)=N%2;
N=Q;
}
stamparesti(resti,n);
ptr2Set=malloc(sizeof(int));
if(ptr2Set==NULL)
{
printf("Allocazione di memoria non riuscita\n");
free(ptr2Set);
exit(1);
}
card=0;
for(j=0;j<n;j++)
{
printf("j=%d card=%d\t",j,card);
if(*(resti+j)==1)
{
ptr2Set=realloc(ptr2Set,++card);
if(ptr2Set==NULL)
{
printf("Allocazione di memoria non riuscita\n");
free(ptr2Set);
exit(1);
}
*(ptr2Set+card-1)=*(set+j);
}
}
free(resti);
(ptrPDS+i)->elementi=card;
(ptrPDS+i)->ptrSet=ptr2Set;
printf("i=%d\telementi=%d\tptrSet=%p\n",i,card,ptr2Set);
}
// Stampa elementi sottoinsieme.
for(i=0;i<m;i++)
{
ptr2Set=(ptrPDS+i)->ptrSet;
printf("S%d={",i);
card=(ptrPDS+i)->elementi;
if (card==0) printf("}\n");
else
{
for(j=0;j<card-1;j++) printf("%d,",*(ptr2Set+j));
printf("%d}\n",*(ptr2Set+j));
}
printf("\n");

}
free(ptrPDS);
free(ptr2Set);
free(set);
return 0;
}
// Funzione per il calcolo della potenza di un numero intero n ad un
esponente
int pot(int base,int esponente)
{
int p=1;
int i=0;
while(i++<esponente)
p=p*base;
return p;
}
// Stampa dei resti
void stamparesti( int *resti, int n)
{
int j;
for(j=0;j<n;j++)
{
printf("\t%d",*(resti+j));
}
printf("\n");
}


I use Open Suse Linux 12.2, gcc compiler 4.7.1


Francesco

BartC

unread,
Dec 22, 2012, 3:32:37 PM12/22/12
to
"francesco" <sarch...@wolfland.it> wrote in message
news:50d60828$0$6827$5fc...@news.tiscali.it...

> ptr2Set=malloc(sizeof(int));

Should this be:
ptr2Set=malloc(sizeof(int)*n);
?

(BTW the layout of your code could be improved!)

--
Bartc

Eric Sosman

unread,
Dec 22, 2012, 3:45:33 PM12/22/12
to
On 12/22/2012 2:21 PM, francesco wrote:
> [...]
> ptr2Set=realloc(ptr2Set,++card);

ptr2set = realloc(ptr2set, ++card * sizeof(int));

Better,

ptr2set = realloc(ptr2set, ++card * sizeof *ptr2set);

Other improvements are possible, and there may be other
problems. Start by fixing this one, and see how far you get.

--
Eric Sosman
eso...@comcast-dot-net.invalid

Rosario1903

unread,
Dec 22, 2012, 5:22:42 PM12/22/12
to
On 22 Dec 2012 19:21:12 GMT, francesco <sarch...@wolfland.it>
wrote:

>>
>> You have an error at line 42.
>
>Sorry, this is he code

tu scrivi qualcosa come
v=malloc(200000);
if(v==NULL)
{free(v);
return 0;
}

mentre e sufficiente scrivere
v=malloc(200000);
if(v==NULL) return 0;

poiche' se v == 0 allora v non punta a memoria
da liberare... cmq free(v) quando v==0 non reca danni
solo: perche' scriverlo se non � necessario?

per il resto, quando ho visto m=2^n e m usato come parametro
per malloc ed come parametro per il ciclo for: ho abbandonato...

James Harris

unread,
Dec 22, 2012, 6:51:53 PM12/22/12
to
On Dec 22, 6:09 pm, francesco <sarchiap...@wolfland.it> wrote:
> I get this strange error message
>
> subsets.exe: malloc.c:2451: sYSMALLOc: Assertion `(old_top == (((mbinptr)

An assertion failure in malloc suggests you have previously
overwritten one or more of its internal data structures. They are
often placed between the spaces malloc/realloc gives to you and should
not be touched.

In your code I see

*(ptr2Set+card-1)=*(set+j);

Is this right? It looks like card starts out as zero so this could
refer to the memory before your allocated area that you should not
touch. This may be it - or there may be others.

James

Phil Carmody

unread,
Dec 22, 2012, 6:44:34 PM12/22/12
to
francesco <sarch...@wolfland.it> writes:
> >
> > You have an error at line 42.
>
> Sorry, this is he code
>
> #include<stdlib.h>
> #include<stdio.h>
> typedef struct {
> int *ptrSet;

Fix your indentation (might be a news client issue).

> int elementi;
> }Set;
> int pot(int base,int esponente);
> void stamparesti( int *resti, int n);
> int main()
> {
> Set *ptrPDS=NULL;
> int *ptr2Set=NULL;
> int *resti=NULL;
> int *set=NULL;
> int n,m;
> int i=0,j=0;
> int N,Q=0;
> int card=0;
> printf("Inserisci il numero di elementi di S\n");
> scanf("%d",&n);
> m=pot(2,n);
> printf("L'insieme dei sottoinsiemi di un insieme S ha %d sottoinsiemi
> \n",m);
> ptrPDS=(Set *)malloc(sizeof(Set)*m);
> if(ptrPDS==NULL)
> {
> printf("Impossibile allocare spazio per l'insieme PDS: memoria
> insufficiente\n");
> free(ptrPDS);

pointless

> exit(1);
> }
> set=malloc(sizeof(int)*n);
> // Verifica se esiste abbastanza memoria per l'allocazione dell'insieme
> if(set==NULL)
> {
> printf("Impossibile allocare spazio per l'insieme S: memoria
> insufficiente\n");
> free(set);

pointless

> exit(1);
> }
>
> printf("Iserire gli elementi di S\n");
> for(i=0;i<n;i++)
> {
> printf("\nInserire elemento %d",i);scanf("%d",set+i);
> }
> for(i=0;i<m;i++)
> {
> printf("i=%d\n",i);
> resti=malloc(sizeof(int)*n);
> if(resti==NULL)
> {
> printf("Impossibile allocare spazio per l'insieme S:
> memoria insufficiente\n");
> free(resti);

pointless

> exit(1);
> }
> N=i;
> //Conversione in binario del numero i
> for(j=0;j<n;j++)
> {
> Q=N/2;
> *(resti+j)=N%2;

use array notation

> N=Q;
> }
> stamparesti(resti,n);
> ptr2Set=malloc(sizeof(int));
> if(ptr2Set==NULL)
> {
> printf("Allocazione di memoria non riuscita\n");
> free(ptr2Set);

pointless

> exit(1);
> }
> card=0;
> for(j=0;j<n;j++)
> {
> printf("j=%d card=%d\t",j,card);
> if(*(resti+j)==1)

use array notation

> {
> ptr2Set=realloc(ptr2Set,++card);

2nd parameter wrong - use ++card*sizeof(*ptr2Set) instead.

always store the return value in a temporary variable to check if it's NULL - upon failure, the original block remains unchanged.

that if statement looks odd - why should the decision whether to realloc the block depend on anything apart from the index being addressed, and the current size of the allocated block?

> if(ptr2Set==NULL)
> {
> printf("Allocazione di memoria non riuscita\n");
> free(ptr2Set);

pointless

> exit(1);
> }
> *(ptr2Set+card-1)=*(set+j);

use array notation

> }
> }
> free(resti);
> (ptrPDS+i)->elementi=card;
> (ptrPDS+i)->ptrSet=ptr2Set;

use array notation

> printf("i=%d\telementi=%d\tptrSet=%p\n",i,card,ptr2Set);
> }
> // Stampa elementi sottoinsieme.
> for(i=0;i<m;i++)
> {
> ptr2Set=(ptrPDS+i)->ptrSet;
> printf("S%d={",i);
> card=(ptrPDS+i)->elementi;

use array notation

> if (card==0) printf("}\n");
> else
> {
> for(j=0;j<card-1;j++) printf("%d,",*(ptr2Set+j));
> printf("%d}\n",*(ptr2Set+j));

use array notation

indent/bracket consistently

> }
> printf("\n");
>
> }
> free(ptrPDS);
> free(ptr2Set);
> free(set);
> return 0;
> }
> // Funzione per il calcolo della potenza di un numero intero n ad un
> esponente
> int pot(int base,int esponente)
> {
> int p=1;
> int i=0;
> while(i++<esponente)
> p=p*base;
> return p;
> }
> // Stampa dei resti
> void stamparesti( int *resti, int n)
> {
> int j;
> for(j=0;j<n;j++)
> {
> printf("\t%d",*(resti+j));

use array notation

> }
> printf("\n");
> }
>
> I use Open Suse Linux 12.2, gcc compiler 4.7.1
>
>
> Francesco


Phil

--
I'm not saying that google groups censors my posts, but there's a strong link
between me saying "google groups sucks" in articles, and them disappearing.

Oh - I guess I might be saying that google groups censors my posts.

francesco

unread,
Dec 22, 2012, 9:20:10 PM12/22/12
to

>> {
>> ptr2Set=realloc(ptr2Set,++card);
>
> 2nd parameter wrong - use ++card*sizeof(*ptr2Set) instead.

yes, it fixed the problem. I have already fixed this sort of problems,
but now I wasn't able to see the solution :)

>
> always store the return value in a temporary variable to check if it's
> NULL - upon failure, the original block remains unchanged.

Ok
>
> that if statement looks odd - why should the decision whether to realloc
> the block depend on anything apart from the index being addressed, and
> the current size of the allocated block?
Do you mean this uf statement ?

if(*(resti+j)==1)

I need to create sets of variable size, and the size depends upon this
test, if rest==1 I need to add an item to the set, else I don't.

{
ptr2Set=realloc(ptr2Set,++card*sizeof(int));
if(ptr2Set==NULL)
{
printf("Allocazione di memoria non riuscita\n");
free(ptr2Set);
exit(1);
}
*(ptr2Set+card-1)=*(set+j);
}

francesco

unread,
Dec 22, 2012, 9:22:22 PM12/22/12
to
Il Sat, 22 Dec 2012 15:45:33 -0500, Eric Sosman ha scritto:

> On 12/22/2012 2:21 PM, francesco wrote:
>> [...]
>> ptr2Set=realloc(ptr2Set,++card);
>
> ptr2set = realloc(ptr2set, ++card * sizeof(int));

yes, this fixed the program, it is always a subtle error
>
> Better,
>
> ptr2set = realloc(ptr2set, ++card * sizeof *ptr2set);
>
> Other improvements are possible, and there may be other
> problems. Start by fixing this one, and see how far you get.

Ok, I fixed that subtle error and the program works now.

Francesco

francesco

unread,
Dec 22, 2012, 9:24:41 PM12/22/12
to

>
> per il resto, quando ho visto m=2^n e m usato come parametro per malloc
> ed come parametro per il ciclo for: ho abbandonato...

Devo allocare un blocco di m strutture,e poi riempirle. Quindi mi serve
sia malloc con m come parametro sia ripetere il ciclo di for m volte

Francesco

Ike Naar

unread,
Dec 23, 2012, 10:59:48 AM12/23/12
to
There is a malloc/free mismatch here.
You allocate n regions of memory, using ptr2Set=malloc(/*...*/) in a loop,
but only one of them is freed at the end of the program.

> free(set);
> return 0;
> }

pete

unread,
Dec 23, 2012, 4:19:10 PM12/23/12
to
Ike Naar wrote:
>
> On 2012-12-22, francesco <sarch...@wolfland.it> wrote:

> > for(i=0;i<m;i++)
> > {

> > (ptrPDS+i)->elementi=card;
> > (ptrPDS+i)->ptrSet=ptr2Set;
> > printf("i=%d\telementi=%d\tptrSet=%p\n",i,card,ptr2Set);
/*
** I think that the free(ptr2Set); statement
** should be relocated to right here,
** to avoid the {allocation / free} mismatch,
** which was mentioned by Ike Naar.
*/
> > }
> > // Stampa elementi sottoinsieme.
> > for(i=0;i<m;i++)
> > {
> > ptr2Set=(ptrPDS+i)->ptrSet;
> > printf("S%d={",i);
> > card=(ptrPDS+i)->elementi;
> > if (card==0) printf("}\n");
> > else
> > {
> > for(j=0;j<card-1;j++) printf("%d,",*(ptr2Set+j));
> > printf("%d}\n",*(ptr2Set+j));
> > }
> > printf("\n");
> >
> > }
> > free(ptrPDS);
> > free(ptr2Set);


> There is a malloc/free mismatch here.
> You allocate n regions of memory, using ptr2Set=malloc(/*...*/)
> in a loop,
> but only one of them is freed at the end of the program.
>
> > free(set);
> > return 0;
> > }

--
pete

pete

unread,
Dec 23, 2012, 4:22:06 PM12/23/12
to
card is incremented as a function call argument
prior to that line.

I don't increment arguments in function calls,
because I've been advised against it
as a matter of style.

--
pete

pete

unread,
Dec 23, 2012, 5:12:01 PM12/23/12
to
After thinking more,
I think that it makes more sense
to allocate (ptr2Set) prior to the loop,
and to then free it after the loop.

And it also makes more sense
to allocate (resti) prior to the loop,
and to then free it after the loop.

puts("Inserisci il numero di elementi di S");
scanf("%d", &n);
m = pot(2, n);
printf("L'insieme dei sottoinsiemi di un insieme S ha %d"
" sottoinsiemi\n", m);
ptrPDS = malloc(m * sizeof *ptrPDS);
set = malloc(n * sizeof *set);
resti = malloc(n * sizeof *resti);
ptr2Set = malloc(sizeof *ptr2Set);
/*
** Verifica se esiste abbastanza
** memoria per l'allocazione dell'insieme
*/
if (ptrPDS == NULL || set == NULL
|| resti == NULL || ptr2Set == NULL)
{
free(resti);
free(set);
free(ptrPDS);
free(ptr2Set);
puts("Allocazione di memoria non riuscita");
exit(EXIT_FAILURE);
}
puts("Iserire gli elementi di S");
for (i = 0; n > i; ++i) {
printf("\nInserire elemento %d ", i);
fflush(stdout);
scanf("%d", set + i);
}
for (i = 0; m > i; ++i) {
printf("i=%d ", i);
N = i;
/*
** Conversione in binario del numero i
*/
for (j = 0; n > j; ++j) {
Q = N / 2;
resti[j] = N % 2;
N = Q;
}
stamparesti(resti, n);
card = 0;
for (j = 0; n > j; ++j) {
printf("j=%d card=%d\n", j, card);
if (resti[j] == 1) {
void *temp;

temp = realloc(ptr2Set, (card + 1) * sizeof *ptr2Set);
if (temp == NULL) {
free(ptr2Set);
free(resti);
free(set);
free(ptrPDS);
puts("Allocazione di memoria non riuscita");
exit(EXIT_FAILURE);
}
ptr2Set = temp;
ptr2Set[card++] = set[j];
}
}
ptrPDS[i].elementi = card;
ptrPDS[i].ptrSet = ptr2Set;
printf("elementi=%d ptrSet=%p\n\n\n\n", card, ptr2Set);
}
free(ptr2Set);
free(resti);
free(set);

--
pete

francesco

unread,
Dec 24, 2012, 2:05:02 AM12/24/12
to
As I did it

>
> And it also makes more sense to allocate (resti) prior to the loop, and
> to then free it after the loop.

>
> puts("Inserisci il numero di elementi di S"); scanf("%d", &n);
> m = pot(2, n);
> printf("L'insieme dei sottoinsiemi di un insieme S ha %d"
> " sottoinsiemi\n", m);
> ptrPDS = malloc(m * sizeof *ptrPDS);
> set = malloc(n * sizeof *set);
> resti = malloc(n * sizeof *resti);
resti is an array of int values, dinamically created
> ptr2Set = malloc(sizeof *ptr2Set);
ptr2Set is an array of int values, dinamically created
Thak you for your advices.

Francesco

Ralph Spitzner

unread,
Dec 24, 2012, 9:51:14 AM12/24/12
to
Phil Carmody wrote:
[...]
>> if(set==NULL)
>> {
>> printf("Impossibile allocare spazio per l'insieme S: memoria
>> insufficiente\n");
>> free(set);
>
> pointless
>

Apart from it being pointless he's actually trying to free a
NULL pointer :-P

-rasp



--
Where's that fucking 'any' key ?

Richard Damon

unread,
Dec 24, 2012, 10:04:39 AM12/24/12
to
On 12/24/12 9:51 AM, Ralph Spitzner wrote:
> Phil Carmody wrote:
> [...]
>>> if(set==NULL)
>>> {
>>> printf("Impossibile allocare spazio per l'insieme S: memoria
>>> insufficiente\n");
>>> free(set);
>>
>> pointless
>>
>
> Apart from it being pointless he's actually trying to free a
> NULL pointer :-P
>
> -rasp

It is perfectly legal to free a null pointer. The free() function has
defined behavior in this case, it does nothing.

The fact that he KNOWS it is a null pointer, is what makes the call
pointless, it just becomes a waste of a few CPU cycles.

James Kuyper

unread,
Dec 24, 2012, 10:16:52 AM12/24/12
to
On 12/24/2012 09:51 AM, Ralph Spitzner wrote:
> Phil Carmody wrote:
> [...]
>>> if(set==NULL)
>>> {
>>> printf("Impossibile allocare spazio per l'insieme S: memoria
>>> insufficiente\n");
>>> free(set);
>>
>> pointless
>>
>
> Apart from it being pointless he's actually trying to free a
> NULL pointer :-P

"NULL" is the name of a standard macro that's required to expand into a
null pointer constant - due to one of the quirks of C jargon, a null
pointer constant is not necessarily a pointer, null or otherwise; it can
be an integer. He's calling free(set), not free(NULL). In the comparison
between set and NULL, NULL gets converted to a null pointer of the same
type as "set"; if that branch has been entered, they compared equal,
which means that "set" is also a null pointer. The word you should have
used was "null", not "NULL".
--
James Kuyper

James Harris

unread,
Dec 24, 2012, 11:59:25 AM12/24/12
to
On Dec 23, 9:22 pm, pete <pfil...@mindspring.com> wrote:
> James Harris wrote:
>
> > On Dec 22, 6:09 pm, francesco <sarchiap...@wolfland.it> wrote:
> > > I get this strange error message
>
> > > subsets.exe: malloc.c:2451: sYSMALLOc: Assertion `(old_top == (((mbinptr)
>
> > An assertion failure in malloc suggests you have previously
> > overwritten one or more of its internal data structures. They are
> > often placed between the spaces malloc/realloc gives to you and should
> > not be touched.
>
> > In your code I see
>
> >   *(ptr2Set+card-1)=*(set+j);
>
> > Is this right? It looks like card starts out as zero so this could
> > refer to the memory before your allocated area that you should not
> > touch. This may be it - or there may be others.
>
> card is incremented as a function call argument
> prior to that line.

Sure, you're right. I didn't want to spend the time to understand the
OP's specific code so I made that part of my post a question rather
than a statement. It was meant to illustrate the potential issue
rather than be the specific cause. The main point of the post was the
part about corrupting malloc/realloc data structures.

> I don't increment arguments in function calls,
> because I've been advised against it
> as a matter of style.

Interesting. I can't see a problem with it.

James

Phil Carmody

unread,
Dec 25, 2012, 1:44:09 PM12/25/12
to
Ralph Spitzner <ra...@spitzner.org> writes:
> Phil Carmody wrote:
> [...]
> >> if(set==NULL)
> >> {
> >> printf("Impossibile allocare spazio per l'insieme S: memoria
> >> insufficiente\n");
> >> free(set);
> >
> > pointless
> >
>
> Apart from it being pointless he's actually trying to free a
> NULL pointer :-P

No, that *is* the pointless bit. It's nothing more than pointless though.
(Apart from it also being a warning sign about what to expect elsewhere
in the code.)

army1987

unread,
Dec 27, 2012, 5:52:50 PM12/27/12
to
On Sat, 22 Dec 2012 20:32:37 +0000, BartC wrote:

> (BTW the layout of your code could be improved!)

I suspect he's using tab characters to indent, but something along the
line (his newsreader, his news server, your news server, your newsreader)
strips them away.



--
[ T H I S S P A C E I S F O R R E N T ]
Troppo poca cultura ci rende ignoranti, troppa ci rende folli.
-- fathermckenzie di it.cultura.linguistica.italiano
<http://xkcd.com/397/>
0 new messages