pointers to structures

Discussion in 'C Programming' started by R Dow, Jul 6, 2006.

  1. R Dow

    R Dow Guest

    I am a bit confused here, having not programmed for some time. Any help
    would be gratefully received.

    If I define a couple of structures thus:

    struct sfinfo {

    double srate;
    short ssize;
    short chans;
    };

    typedef struct {
    void *pRawBuf;
    FILE *fp;
    struct sfinfo info;
    } SOUNDFILE;

    and a function so:

    void sfinfocopy(struct sfinfo *from, struct sfinfo *to) {

    to->srate = from->srate;
    to->ssize = from->ssize;
    to->chans = from->chans;

    }

    If I then do something like this:

    blah(struct sfinfo *info) {

    SOUNDFILE *outsfp;

    outsfp = (SOUNDFILE *)malloc(sizeof(SOUNDFILE)))==NULL;

    sfinfocopy(info, &(outsfp->info));

    }

    It doesn't seem to work. The expression &(outsfp->info) isn't right, is it?


    Robert
    R Dow, Jul 6, 2006
    #1
    1. Advertising

  2. The following compiles just fine for me:

    #include <stdio.h>

    struct sfinfo {

    double srate;
    short ssize;
    short chans;

    };

    typedef struct {

    void *pRawBuf;
    FILE *fp;
    struct sfinfo info;

    } SOUNDFILE;


    void sfinfocopy(struct sfinfo *from, struct sfinfo *to) {

    to->srate = from->srate;
    to->ssize = from->ssize;
    to->chans = from->chans;

    }


    blah(struct sfinfo *info) {

    SOUNDFILE *outsfp;

    outsfp = (SOUNDFILE *)malloc(sizeof(SOUNDFILE)); /* Remove ERROR */

    sfinfocopy(info, &(outsfp->info));

    }


    int main(void)
    {
    struct sfinfo obj = {0};

    blah(&obj);
    }



    --

    Frederick Gotham
    Frederick Gotham, Jul 6, 2006
    #2
    1. Advertising

  3. R Dow

    Tom St Denis Guest

    R Dow wrote:
    > blah(struct sfinfo *info) {
    >
    > SOUNDFILE *outsfp;
    >
    > outsfp = (SOUNDFILE *)malloc(sizeof(SOUNDFILE)))==NULL;
    >
    > sfinfocopy(info, &(outsfp->info));
    >
    > }
    >
    > It doesn't seem to work. The expression &(outsfp->info) isn't right, is it?


    No it's fine. It's your malloc line.

    1. Don't cast return [hint: include stdlib.h]
    2. use sizeof( *outsfp ) as the size inside
    3. What is ==NULL supposed to do?
    4. Add an if statement after the malloc to check for NULL

    Tom
    Tom St Denis, Jul 6, 2006
    #3
  4. Frederick Gotham posted:


    > outsfp = (SOUNDFILE *)malloc(sizeof(SOUNDFILE));



    #include <stdlib.h>



    --

    Frederick Gotham
    Frederick Gotham, Jul 6, 2006
    #4
  5. R Dow

    Tom St Denis Guest

    Frederick Gotham wrote:

    #include <stdlib.h>

    > blah(struct sfinfo *info) {
    >
    > SOUNDFILE *outsfp;
    >
    > outsfp = malloc(sizeof(*outsfp)); /* Remove ERRORs */


    if (outsfp == NULL) {
    handle_error();
    return;
    }

    > sfinfocopy(info, &(outsfp->info));
    >
    > }


    I fixed YOUR errors (inline and indented).

    Tom
    Tom St Denis, Jul 6, 2006
    #5
  6. R Dow wrote:
    >...
    >void sfinfocopy(struct sfinfo *from, struct sfinfo *to) {
    > to->srate = from->srate;
    > to->ssize = from->ssize;
    > to->chans = from->chans;
    >}
    >...
    >sfinfocopy(info, &(outsfp->info));


    In addition to what others posted, why not just

    info = outsfp->info;

    instead of calling a function?

    >outsfp = (SOUNDFILE *)malloc(sizeof(SOUNDFILE)))==NULL;


    There is one right parenthesis too many. You should get a compile
    error here.
    Roberto Waltman, Jul 6, 2006
    #6
  7. R Dow

    R Dow Guest

    Thanks for that. In fact, I tried to extract the example from the actual
    code to make it easier to read, and thus introduced errors. So I
    tried testing the extract and the extract works fine, so there is
    obviously a far nastier error causing the problems.

    Thanks again

    Robert
    R Dow, Jul 6, 2006
    #7
  8. R Dow

    Suman Guest

    R Dow wrote:
    > I am a bit confused here, having not programmed for some time. Any help
    > would be gratefully received.
    >
    > If I define a couple of structures thus:
    >
    > struct sfinfo {
    >
    > double srate;
    > short ssize;
    > short chans;
    > };
    >
    > typedef struct {
    > void *pRawBuf;
    > FILE *fp;
    > struct sfinfo info;
    > } SOUNDFILE;


    A bit strange, since you use typedef for one struct and leave the
    other as is: consistency is IMHO *a good thing* in the long run.

    Also, all uppercase identifiers are generally used for macros, so
    you might want to look at that again, as well.

    > and a function so:
    >
    > void sfinfocopy(struct sfinfo *from, struct sfinfo *to) {


    I would be picky to say this, but still, the first parameter should
    ideally be const-qualified:
    void sfinfocopy(const struct sfinfo *from, struct sfinfo *to) {
    A design choice really, to indicate that this is read-only in
    sfinfocopy.

    > to->srate = from->srate;
    > to->ssize = from->ssize;
    > to->chans = from->chans;
    >
    > }


    This is redundant. For such simple structures, assignment is good
    enough.It wasn't there always, but has been for a while now, so you
    can write:
    struct sfinfo to, from;
    /* fill out from */
    ...
    to = from;
    > If I then do something like this:
    >
    > blah(struct sfinfo *info) {


    Needs a return type. The implicit int is not a good choice anymore.
    Or, put in void, if you'd not want to return anything back to the
    caller.

    > SOUNDFILE *outsfp;
    >
    > outsfp = (SOUNDFILE *)malloc(sizeof(SOUNDFILE)))==NULL;


    A few problems here. Let's take them apart one by one.
    [1] #include <stdlib.h> is required to bring malloc() in scope
    [2] the cast is unnecessary except for *very* few cases (your's doesn't
    look like one till now, so ...)
    [3] the comparison with NULL: comparison returns 1(if true) or 0
    (if false), always. So, here, outsfp becomes either 0 or 1 and
    [a] when it's zero you don't chek it and trying to access info
    through outsfp is Undefined Behavior and will possibly
    land you with a crash
    when it's one, you are trying to use 1, an integer as the
    address of an object of type SOUNDFILE, which on most
    systems will be an error. Integers and pointers are ,
    interchangeable but the result is implementation defined
    and it is very likely you can't have an object placed at
    such an address, let alone use it.

    So, ideally you should've written:[*]
    outsfp = malloc(sizeof *outsfp);
    and taken the trouble of testing it like this:
    if ( outsfp == NULL ) {
    die_horribly();
    /* return error code, probably */
    }
    and gone on to use it when and where required.
    You'd need to give outsfp some valid state first so that the copy
    makes sense. So fill it in:
    outsfp->pRawBuf = malloc( /*some memory, maybe?*/ );
    outsfp->fp = someSndfp;
    ...

    > sfinfocopy(info, &(outsfp->info));


    This is where nasal demons start flying out ;-) See above.

    > }
    >
    > It doesn't seem to work. The expression &(outsfp->info) isn't right, is it?


    No. See above.


    [*] A better approach would be to club the two in one statement
    -- RAII (IMHO is a good thing), like:
    SOUNDFILE *outsfp = malloc(sizeof *outsfp);
    /* so do we have something here? */
    if ( ... ) {
    ...
    HTH
    --
    Suman
    Suman, Jul 6, 2006
    #8
  9. R Dow

    Default User Guest

    R Dow wrote:

    > Thanks for that. In fact, I tried to extract the example from the
    > actual code to make it easier to read, and thus introduced errors. So
    > I tried testing the extract and the extract works fine, so there is
    > obviously a far nastier error causing the problems.


    You've ably demonstrated why we want to see complete, minimal programs
    that reproduce the error.




    Brian
    Default User, Jul 6, 2006
    #9
  10. You don't need a function to copy a struct, the regular assignment
    operator works just fine:

    to = from;

    ----

    The problem is in your malloc:

    > outsfp = (SOUNDFILE *)malloc(sizeof(SOUNDFILE)))==NULL;


    the ==NULL business sets outsfp to false or true, not a good pointer.

    so try:

    outsfp = (SOUNDFILE *) malloc( sizeof(SOUNDFILE) ));

    if( outsfp != NULL ) outsfp->info = incoming;
    Ancient_Hacker, Jul 6, 2006
    #10
  11. "Ancient_Hacker" <> writes:
    > You don't need a function to copy a struct, the regular assignment
    > operator works just fine:
    >
    > to = from;
    >
    > ----
    >
    > The problem is in your malloc:
    >
    >> outsfp = (SOUNDFILE *)malloc(sizeof(SOUNDFILE)))==NULL;

    >
    > the ==NULL business sets outsfp to false or true, not a good pointer.
    >
    > so try:
    >
    > outsfp = (SOUNDFILE *) malloc( sizeof(SOUNDFILE) ));
    >
    > if( outsfp != NULL ) outsfp->info = incoming;


    Much better:

    outsfp = malloc(sizeof *outsfp);
    if (outsfp == NULL) {
    /* error handling */
    }
    ....


    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
    We must do something. This is something. Therefore, we must do this.
    Keith Thompson, Jul 6, 2006
    #11
  12. R Dow

    Jesper Guest

    Frederick Gotham wrote:
    > Frederick Gotham posted:
    > > outsfp = (SOUNDFILE *)malloc(sizeof(SOUNDFILE));

    >
    > #include <stdlib.h>
    >


    .... and free(outsfp); somewhere.
    Jesper, Jul 7, 2006
    #12
    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. Replies:
    7
    Views:
    413
  2. DSKR

    Pointers to 'Data Structures' in C

    DSKR, Jun 24, 2003, in forum: C Programming
    Replies:
    31
    Views:
    10,053
    Giuseppe
    Jul 3, 2003
  3. tweak
    Replies:
    14
    Views:
    2,769
    Eric Sosman
    Jun 11, 2004
  4. Alfonso Morra
    Replies:
    11
    Views:
    703
    Emmanuel Delahaye
    Sep 24, 2005
  5. cerr

    pointers, pointers, pointers...

    cerr, Apr 7, 2011, in forum: C Programming
    Replies:
    12
    Views:
    657
Loading...

Share This Page