structure malloc help

Discussion in 'C Programming' started by mdrons@yahoo.com, Jul 29, 2006.

  1. Guest

    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
     
    , Jul 29, 2006
    #1
    1. Advertising

  2. schrieb:
    > 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.

    --
    Thomas
     
    Thomas J. Gritzan, Jul 29, 2006
    #2
    1. Advertising

  3. Old Wolf Guest

    wrote:
    > 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.
     
    Old Wolf, Jul 29, 2006
    #3
  4. Bill Pursell Guest

    wrote:
    >

    <snip>
    > 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.

    <snip>
    > 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.
     
    Bill Pursell, Jul 29, 2006
    #4
  5. Mike Guest

    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
     
    Mike, Jul 30, 2006
    #5
  6. Bill Pursell Guest

    Mike wrote:
    > 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.

    <snip>
    >
    > qsort(&sqlprog,sqlcount,sizeof(*sqlprog),schedule_compare);

    <snip>

    That should be: qsort(sqlprog, ...)
    Drop the '&'.
     
    Bill Pursell, Jul 30, 2006
    #6
  7. Old Wolf Guest

    Mike wrote:
    >
    > 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;
     
    Old Wolf, Jul 31, 2006
    #7
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. John
    Replies:
    13
    Views:
    721
  2. ravi
    Replies:
    0
    Views:
    469
  3. Peter
    Replies:
    34
    Views:
    2,018
    Richard Tobin
    Oct 22, 2004
  4. porting non-malloc code to malloc

    , Feb 18, 2005, in forum: C Programming
    Replies:
    3
    Views:
    491
    Walter Roberson
    Feb 19, 2005
  5. Johs32

    to malloc or not to malloc??

    Johs32, Mar 30, 2006, in forum: C Programming
    Replies:
    4
    Views:
    334
    Captain Winston
    Mar 30, 2006
Loading...

Share This Page