pointers to structures

[R Dow]

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

 

[Frederick Gotham]

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);
}

 

[Tom St Denis]

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

 

[Frederick Gotham]

Frederick Gotham posted:

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

#include <stdlib.h>

 

[Tom St Denis]

Frederick Gotham wrote:

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

 

[Roberto Waltman]

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

 

[R Dow]

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

 

[Suman]

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

 

[Default User]

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

 

[Ancient_Hacker]

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;

 

[Keith Thompson]

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:


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 */
}
....

 

[Jesper]

Frederick Gotham posted:

#include <stdlib.h>

.... and free(outsfp); somewhere.