structure malloc help

[mdrons]

I have a structure defined as:
struct channel {
int chanid;
int channum;
char callsign[20];
char name[64];
};

I then have a global variable defined as:
static struct channel *chan=NULL;

I then malloc the space when I know the total number of rows (dynamic
variable Total_rows) I have:
if ( (chan=(struct channel*)malloc(sizeof(struct channel)*Total_rows))
=
= NULL) {
snprintf(buf, sizeof(buf),"Mallor Error.\n" );
free (chan);
goto out;
}

My issue as I fill the structure the program seg faults. If I change
the line to
if ( (chan=(struct channel*)malloc(sizeof(struct
channel)*Total_rows*sizeof(struct channel))) =
= NULL) {

It works, but I now have sizeof twice.... or if I make the Total_rows
variable 3 times as large.

Can anyone tell me what I am doing wrong?

Thanks Mike

 

[Thomas J. Gritzan]

I have a structure defined as:
struct channel {
int chanid;
int channum;
char callsign[20];
char name[64];
};

I then have a global variable defined as:
static struct channel *chan=NULL;

I then malloc the space when I know the total number of rows (dynamic
variable Total_rows) I have:
if ( (chan=(struct channel*)malloc(sizeof(struct channel)*Total_rows))

Don't cast malloc's return value. Its bad as it might hide a missing
#include and its not necessary in C.

=
= NULL) {
snprintf(buf, sizeof(buf),"Mallor Error.\n" );
free (chan);

Is there a reason to free(chan) when it's NULL?

goto out;
}

My issue as I fill the structure the program seg faults. If I change
the line to
if ( (chan=(struct channel*)malloc(sizeof(struct
channel)*Total_rows*sizeof(struct channel))) =
= NULL) {

It works, but I now have sizeof twice.... or if I make the Total_rows
variable 3 times as large.

Can anyone tell me what I am doing wrong?

Your code that fills the struct is wrong. Or it might be wrong. Can't
say because you didn't show it.

 

[Old Wolf]

I have a structure defined as:
struct channel {
int chanid;
int channum;
char callsign[20];
char name[64];
};

I then have a global variable defined as:
static struct channel *chan=NULL;

I then malloc the space when I know the total number of rows (dynamic
variable Total_rows) I have:
if ( (chan=(struct channel*)malloc(sizeof(struct channel)*Total_rows))
=
= NULL) {

chan = malloc( Total_rows * sizeof *chan );

Don't cast the value returned by malloc -- it serves no purpose,
and can even hide compiler warnings in some cases.

Also, using "sizeof *chan" guarantees you of allocation the right
amount of space, no matter what.

snprintf(buf, sizeof(buf),"Mallor Error.\n" );
free (chan);

If you got this far, then chan is NULL, so there's no need to free it.

goto out;
}

My issue as I fill the structure the program seg faults. If I change
the line to
if ( (chan=(struct channel*)malloc(sizeof(struct
channel)*Total_rows*sizeof(struct channel))) =
= NULL) {

It works, but I now have sizeof twice....

Can anyone tell me what I am doing wrong?

Somewhere else in the code, you have a buffer overflow. Probably
over the end of 'chan' but not necessarily.

Try to reduce the program to a smaller example that you can
post in its entirety, and does demonstrate the problem.

 

[Bill Pursell]

I then malloc the space when I know the total number of rows (dynamic
variable Total_rows) I have:
if ( (chan=(struct channel*)malloc(sizeof(struct channel)*Total_rows))
=
= NULL) {
snprintf(buf, sizeof(buf),"Mallor Error.\n" );
free (chan);
goto out;
}

My issue as I fill the structure the program seg faults.

Can anyone tell me what I am doing wrong?

As it is, it doesn't look like there is anything wrong with
this code that would cause the segfault. Casting the
return value of malloc is a bad idea, using "sizeof *chan"
is cleaner, and you can probably make a more graceful
exit without the free and by doing something like
err_message = "Out of Memory"; (or using perror() and
relying on the system to set errno), but none of these
will cause the error you're describing. The fault lies
elsewhere. Here are 2 hints to find the problem, both
of these assume you are running on Linux. (I'm going
to give specific commands which work on linux, but the
2 principles will apply to any system.)

1) Let the system check memory consistency for you.
Either run your program through something like valgrind
or electric fence or use mcheck. Read the info page
for mcheck and examine /usr/include/mcheck.h. Basically,
If you add mcheck(NULL); at the start of the program
and an occasional call to mcheck_check_all(), it will
help locate the problem.

2) Examine the stack trace to help pinpoint the problem:
% gcc -g foo.c
% gdb a.out
(gdb) run
(gdb) bt
The bt command gives a backtrace and shows the line
that was executing when the segfault occurs.

Memory errors can be a pain, and the error is usually
not where you expect it to be.

 

[Mike]

Thank you everyone. for the help.

I still have not figured out why the malloc is not correct. But I have
figured out why it is seg faulting.

It is the qsort line below. I have provided more code.

if ( (chan=malloc(sizeof(struct channel)*Total_rows*4)) == NULL) {
cmyth_dbg(CMYTH_DBG_DEBUG, "%s [%s:%d]: (trace) -1)\n",
__FUNCTION__, __FILE__, __LINE__);
snprintf(buf, sizeof(buf),"Mallor Error.\n" );
mvpw_add_menu_item(widget, buf , (void*)i, &item_attr);
goto out;
}

if
(myth_load_channels(&chan,mysqlptr->host,mysqlptr->user,mysqlptr->pass,mysqlptr->db)
< 0) {
cmyth_dbg(CMYTH_DBG_DEBUG, "%s [%s:%d]: (trace) -1)\n",
__FUNCTION__, __FILE__, __LINE__);
snprintf(buf, sizeof(buf),"Database Error. Please
check your settings\n" );
mvpw_add_menu_item(widget, buf , (void*)i, &item_attr);
goto out;
} /* this function fills the chan struct. */

if ((sqlprog=malloc(sizeof(struct program)*10))==NULL) {
perror("malloc");
cmyth_dbg(CMYTH_DBG_DEBUG, "%s [%s:%d]: (trace) -1)\n",
__FUNCTION__, __FILE__, __LINE__);
snprintf(buf, sizeof(buf),"Mallor Error.\n" );
mvpw_add_menu_item(widget, buf , (void*)i, &item_attr);
free (sqlprog);
goto out;
} /* this is a program structure.

sqlcount=get_guide_mysql(&sqlprog,chan,starttime,endtime,mysqlptr->host,
mysqlptr->user, mysqlptr->pass,mysqlptr->db); /* this fills the
program structure */

qsort(&sqlprog,sqlcount,sizeof(*sqlprog),schedule_compare);


static int
schedule_compare (const void *a, const void *b)
{
const struct program *x, *y;
int X, Y;

x = ((const struct program*)a);
y = ((const struct program*)b);
X = x->channum;
Y = y->channum;

if (X < Y) {
return -1;
}
else if (X > Y) {
return 1;
}
else {
return 0;
}
}

Any thoughts as to what I am doing wrong with the qsort? Thank you.
-Mike

 

[Bill Pursell]

Thank you everyone. for the help.

I still have not figured out why the malloc is not correct. But I have
figured out why it is seg faulting.

It is the qsort line below. I have provided more code.

qsort(&sqlprog,sqlcount,sizeof(*sqlprog),schedule_compare);

<snip>

That should be: qsort(sqlprog, ...)
Drop the '&'.

 

[Old Wolf]

I still have not figured out why the malloc is not correct. But I have
figured out why it is seg faulting.

The malloc line is correct (without the "*4"), assuming you
have #included <stdlib.h>.

But it is fragile. Please follow the advice about malloc in my
previous post.

It is the qsort line below. I have provided more code.

if ((sqlprog=malloc(sizeof(struct program)*10))==NULL) {
perror("malloc");
cmyth_dbg(CMYTH_DBG_DEBUG, "%s [%s:%d]: (trace) -1)\n",
__FUNCTION__, __FILE__, __LINE__);
snprintf(buf, sizeof(buf),"Mallor Error.\n" );
mvpw_add_menu_item(widget, buf , (void*)i, &item_attr);
free (sqlprog);
goto out;
} /* this is a program structure.

sqlcount=get_guide_mysql(&sqlprog,chan,starttime,endtime,mysqlptr->host,
mysqlptr->user, mysqlptr->pass,mysqlptr->db); /* this fills the
program structure */

You only allocated space for 10 structs to 'sqlprog'. How do you
make sure that 'get_guide_mysql' does not overflow this?

qsort(&sqlprog,sqlcount,sizeof(*sqlprog),schedule_compare);

Should be:
qsort( sqlprog, sqlcount, sizeof *sqlprog, schedule_compare );

sqlprog is a pointer to the first thing you want to sort, and that is
what qsort is expecting. Not the address of a pointer to the things
to sort.

I suspect you also have an extra & in the call to get_guide_mysql,
although it is impossible to say without seeing the explanation of
how that function works.

static int
schedule_compare (const void *a, const void *b)
{
const struct program *x, *y;
int X, Y;

x = ((const struct program*)a);
y = ((const struct program*)b);

Useless casts. It is good to not use casts unless absolutely necessary.
Instead, write:
x = a;
y = b;