«

I have structures like this in my program -

typedef vector vectorstruct
{
double x, y,z;
} vector;

typedef struct verticesstruct
{
vector v;
} vertex; /*a vertex is a vector */

typedef struct trianglestruct
{
int v0,v1, v2;
}triangle;

typedef struct objectstruct
{
int nvert;
int ntri;
vertex *vert;
triangle *tri;
}object;

................
...............

later somewhere in the program i have a statement like this -

object *obj;

obj->vert = malloc( nvert * sizeof(vertex)); /* Creating an array of
vertices */

obj->tri = malloc(ntri * sizeof(triangle));

for(i=0;i<obj->nvert; i++)/* trying to take input for each vertex
which has x, y, z components as it is a vector*/
scanf("%f %f %f", &obj->vert[i].x, &obj->vert[i].y, &obj->vert[i].z);

^^^^^^^^ is this above notation of creating an array and then
accessing the elements correct ??


later on i also do this -

for(i=0;i<obj->ntri;i++)
scanf("%d %d %d", &obj->tri[i].v0, &obj->tri[i].v1, &obj->tri[i].v2);

On Sun, 09 Mar 2008 06:45:54 -0700, Cell wrote:
> I have structures like this in my program -
>
> typedef vector vectorstruct

Syntax error. "vector" should be "struct". Please try to copy and paste
from code you've actually tried to use as much as possible, so that errors
such as these can be avoided.

> {
> double x, y,z;
> } vector;
>
> typedef struct verticesstruct
> {
> vector v;

This is fine, but...

> } vertex; /*a vertex is a vector */

....vertex is a structure with one member. This member is named "v", and
has type vector.

> typedef struct trianglestruct
> {
> int v0,v1, v2;
> }triangle;

Okay, so a triangle has three members v0, v1, and v2.

> typedef struct objectstruct
> {
> int nvert;
> int ntri;
> vertex *vert;
> triangle *tri;
> }object;
>
> ...............
> ..............
>
> later somewhere in the program i have a statement like this -
>
> object *obj;
>
> obj->vert = malloc( nvert * sizeof(vertex)); /* Creating an array of
> vertices */

What is nvert? You haven't declared it. Did you mean obj->nvert? Please
try to copy and paste from code you've actually tried to use as much as
possible, so that errors such as these can be avoided.

> obj->tri = malloc(ntri * sizeof(triangle));

See above.

> for(i=0;i<obj->nvert; i++)/* trying to take input for each vertex which
> has x, y, z components as it is a vector*/ scanf("%f %f %f",
> &obj->vert[i].x, &obj->vert[i].y, &obj->vert[i].z);

A vertex has one member. This member has a name "v". A vertex does not
have any members x, y, or z. A vector does.

> ^^^^^^^^ is this above notation of creating an array

The array creation is okay.

> and then accessing
> the elements correct ??

The access is not okay.

> later on i also do this -
>
> for(i=0;i<obj->ntri;i++)
> scanf("%d %d %d", &obj->tri[i].v0, &obj->tri[i].v1, &obj->tri[i].v2);

A triangle has members v0, v1, and v2, and you're accessing members v0,
v1, and v2. Not surprisingly, this works.

Cell wrote:
> I have structures like this in my program -

A problem with "like this" code fragments is that when
someone sees a problem with them and then tries to explain to
you the nature of the problem, it may have nothing to do with
the actual problems of the actual original code.

Long ago when kings were regarded as semi-divine, it was
seen that their divinity did not shield them from illness and
they sometimes needed treatment from doctors. But most doctors
were not of noble blood, and in some places it was unthinkable
for a commoner to touch the Royal Person. So the king would
nominate some less-divine person as a surrogate, who would
imitate the king's malady while submitting to the doctor's
examination. The proprieties were respected, but what do you
think of the likely accuracy of such diagnosis by proxy?

> typedef vector vectorstruct

For example, this line is invalid and will not compile.
It serves no purpose except to distract attention from the
question you are actually interested in.

> {
> double x, y,z;
> } vector;
>
> typedef struct verticesstruct
> {
> vector v;
> } vertex; /*a vertex is a vector */

Seems a rather pointless (pun intended) struct; why
bother having it at all?

> typedef struct trianglestruct
> {
> int v0,v1, v2;
> }triangle;
>
> typedef struct objectstruct
> {
> int nvert;
> int ntri;
> vertex *vert;
> triangle *tri;
> }object;
>
> ...............
> ..............
>
> later somewhere in the program i have a statement like this -
>
> object *obj;
>
> obj->vert = malloc( nvert * sizeof(vertex)); /* Creating an array of
> vertices */

You're carefully allocating space for a bunch of vertices,
but you've forgotten to allocate space for the object. The
pointer variable `obj' has not been initialized; it "has a
garbage value." When you try to store a value in `obj->vert',
anything at all might happen.

(Oh, you say you *do* allocate some space and point obj
at it? Well, why didn't you say so in the first place, by
posting actual code instead of "like this" code?)

If we assume that obj actually points to something usable
and that nvert is an integer that is non-negative and not too
huge, then the allocation is all right. It can be improved
just a little bit by writing it as

obj->vert = malloc(nvert * sizeof *obj->vert);

Either way, the allocation attempt should be followed
(usually immediately, but sometimes a little distance is
permissible) by a test to see whether the attempt succeeded:

if (obj->vert == NULL) die_or_take_corrective_action();

> obj->tri = malloc(ntri * sizeof(triangle));

Same remarks.

> for(i=0;i<obj->nvert; i++)/* trying to take input for each vertex
> which has x, y, z components as it is a vector*/
> scanf("%f %f %f", &obj->vert[i].x, &obj->vert[i].y, &obj->vert[i].z);
>
> ^^^^^^^^ is this above notation of creating an array and then
> accessing the elements correct ??

Yes, under reasonable assumptions about what the code
actually looks like. But you're using scanf() incorrectly:
"%f" is the wrong conversion specifier for double, and any
input failure will escape your notice entirely.

> later on i also do this -
>
> for(i=0;i<obj->ntri;i++)
> scanf("%d %d %d", &obj->tri[i].v0, &obj->tri[i].v1, &obj->tri[i].v2);

As before, the array accesses look fine given similar
assumptions, and as before the use of scanf() is broken.

Real code next time, please.

--
Eric Sosman
lid

On Mar 9, 7:21 pm, Eric Sosman <esos...@ieee-dot-org.invalid> wrote:
> Cell wrote:
> > I have structures like this in my program -
>
> A problem with "like this" code fragments is that when
> someone sees a problem with them and then tries to explain to
> you the nature of the problem, it may have nothing to do with
> the actual problems of the actual original code.
>
> Long ago when kings were regarded as semi-divine, it was
> seen that their divinity did not shield them from illness and
> they sometimes needed treatment from doctors. But most doctors
> were not of noble blood, and in some places it was unthinkable
> for a commoner to touch the Royal Person. So the king would
> nominate some less-divine person as a surrogate, who would
> imitate the king's malady while submitting to the doctor's
> examination. The proprieties were respected, but what do you
> think of the likely accuracy of such diagnosis by proxy?
>
> > typedef vector vectorstruct
>
> For example, this line is invalid and will not compile.
> It serves no purpose except to distract attention from the
> question you are actually interested in.
>
> > {
> > double x, y,z;
> > } vector;
>
> > typedef struct verticesstruct
> > {
> > vector v;
> > } vertex; /*a vertex is a vector */
>
> Seems a rather pointless (pun intended) struct; why
> bother having it at all?
>
>
>
>
>
> > typedef struct trianglestruct
> > {
> > int v0,v1, v2;
> > }triangle;
>
> > typedef struct objectstruct
> > {
> > int nvert;
> > int ntri;
> > vertex *vert;
> > triangle *tri;
> > }object;
>
> > ...............
> > ..............
>
> > later somewhere in the program i have a statement like this -
>
> > object *obj;
>
> > obj->vert = malloc( nvert * sizeof(vertex)); /* Creating an array of
> > vertices */
>
> You're carefully allocating space for a bunch of vertices,
> but you've forgotten to allocate space for the object. The
> pointer variable `obj' has not been initialized; it "has a
> garbage value." When you try to store a value in `obj->vert',
> anything at all might happen.
>
> (Oh, you say you *do* allocate some space and point obj
> at it? Well, why didn't you say so in the first place, by
> posting actual code instead of "like this" code?)
>
> If we assume that obj actually points to something usable
> and that nvert is an integer that is non-negative and not too
> huge, then the allocation is all right. It can be improved
> just a little bit by writing it as
>
> obj->vert = malloc(nvert * sizeof *obj->vert);
>
> Either way, the allocation attempt should be followed
> (usually immediately, but sometimes a little distance is
> permissible) by a test to see whether the attempt succeeded:
>
> if (obj->vert == NULL) die_or_take_corrective_action();
>
> > obj->tri = malloc(ntri * sizeof(triangle));
>
> Same remarks.
>
> > for(i=0;i<obj->nvert; i++)/* trying to take input for each vertex
> > which has x, y, z components as it is a vector*/
> > scanf("%f %f %f", &obj->vert[i].x, &obj->vert[i].y, &obj->vert[i].z);
>
> > ^^^^^^^^ is this above notation of creating an array and then
> > accessing the elements correct ??
>
> Yes, under reasonable assumptions about what the code
> actually looks like. But you're using scanf() incorrectly:
> "%f" is the wrong conversion specifier for double, and any
> input failure will escape your notice entirely.
>
> > later on i also do this -
>
> > for(i=0;i<obj->ntri;i++)
> > scanf("%d %d %d", &obj->tri[i].v0, &obj->tri[i].v1, &obj->tri[i].v2);
>
> As before, the array accesses look fine given similar
> assumptions, and as before the use of scanf() is broken.
>
> Real code next time, please.
>
> --
> Eric Sosman
> esos...@ieee-dot-org.invalid

I apologize.

I will post the entire code and provide full details.

please help.

#include<stdio.h>
#include<stdlib.h>
#include<string.h>

typedef struct vectorstruct
{
double x, y, z;
}vector;

typedef struct verticesstruct
{
vector v;
}vertex;

typedef struct trianglestruct
{
int v0, v1, v2;
}triangle;

typedef struct objectstruct
{
int nvert;
int ntri;
vertex *vert;
triangle *tri;
}object;

int read_dat_file( char * dat_file, object * obj )
{

FILE *fp;
int i;

if(!(fp = fopen(dat_file,"r")))
return -1;
while(fp!=NULL)
{
fscanf(fp, "%d %d", &(obj->nvert), &(obj->ntri));
printf("\n%d %d", obj->nvert, obj->ntri);

obj->vert= malloc(obj->nvert * sizeof(vertex));

obj->tri = malloc(obj->ntri * sizeof(triangle));

for(i=0;i<obj->nvert;i++)
{
fscanf(fp, "%f %f %f",&(obj->vert[i].v.x), &(obj->vert[i].v.y), &(obj-
>vert[i].v.z));
printf("%f %f %f", obj->vert[i].v.x, obj->vert[i].v.y, obj-
>vert[i].v.z);
}

for(i=0;i<obj->ntri;i++)
{
fscanf(fp, "%d %d %d",&(obj->tri[i].v0), &(obj->tri[i].v1), &(obj-
>tri[i].v2));
printf("%d %d %d", obj->tri[i].v0, obj->tri[i].v1, obj->tri[i].v2);
}

}
return 1;
}

int main()
{

object *obj;
char *s;
clrscr();
obj = malloc(sizeof(object));
strcpy(s, "sphere.dat");
if(!(read_dat_file(s , obj)))
printf("unsuccessful");
return 0;

}

the sphere.dat file contains description of a sphere which has been
triangulated..

the first line of sphere.dat is something like this -

nvert ntri /*the number of vertices, number of triangles */

in the case of a sphere, nvert = 602, ntri = 1200

from the second line we have coordinates of each vertex. A combination
of 3 vertices forms a triangle.

x1 y1 z1
x2 y2 z2
.........
.........
.........
xnvert ynvert znvert

I stored this in a list of vertices. Hence the vert pointer in object
structure. Similarly there is a tri pointer which can be used for
dynamically creating a list of triangles.

After the n vertices, we have the description of triangles (1200
entries)

12 0 1
3 4 5
23 8 5
.......
.......
.......

Those numbers are basically indices to vertex list. So 12 indicates
the 12th vertex or 12 the entry in the vertex list. 0 indicates
0th(first entry) in the vertex list 1 indicates 1st element of vertex
list. So basically a combination of 3 vertices is forming a triangle.
And this is what Im trying to read from the file and print.

Cell wrote:

> #include<stdio.h>
> #include<stdlib.h>
> #include<string.h>
>
> typedef struct vectorstruct
> {
> double x, y, z;
> }vector;
>
> typedef struct verticesstruct
> {
> vector v;
> }vertex;
>
> typedef struct trianglestruct
> {
> int v0, v1, v2;
> }triangle;
>
> typedef struct objectstruct
> {
> int nvert;
> int ntri;
> vertex *vert;
> triangle *tri;
> }object;
>
> int read_dat_file( char * dat_file, object * obj )
> {
>
> FILE *fp;
> int i;
>
> if(!(fp = fopen(dat_file,"r")))
> return -1;
> while(fp!=NULL)
> {
> fscanf(fp, "%d %d", &(obj->nvert), &(obj->ntri));
> printf("\n%d %d", obj->nvert, obj->ntri);
>
> obj->vert= malloc(obj->nvert * sizeof(vertex));
>
> obj->tri = malloc(obj->ntri * sizeof(triangle));
>
> for(i=0;i<obj->nvert;i++)
> {
> fscanf(fp, "%f %f %f",&(obj->vert[i].v.x), &(obj->vert[i].v.y), &(obj-
>>vert[i].v.z));
> printf("%f %f %f", obj->vert[i].v.x, obj->vert[i].v.y, obj-
>>vert[i].v.z);
> }
>
> for(i=0;i<obj->ntri;i++)
> {
> fscanf(fp, "%d %d %d",&(obj->tri[i].v0), &(obj->tri[i].v1), &(obj-
>>tri[i].v2));
> printf("%d %d %d", obj->tri[i].v0, obj->tri[i].v1, obj->tri[i].v2);
> }
>
> }
> return 1;
> }
>
> int main()
> {
>
> object *obj;
> char *s;
> clrscr();

This is not a portable function. The next compiler you compile it on
might not have such a function.

> obj = malloc(sizeof(object));
> strcpy(s, "sphere.dat");

You have not allocates space for copying.

> if(!(read_dat_file(s , obj)))

Anything other than zero is regarded as "true" in C. Therefore the -1
that you return from read_dat_file will also be regarded as true. The
following printf statement will execute only when read_dat_file returns
zero, which it never does.

> printf("unsuccessful");
> return 0;
>
> }

> This is not a portable function. The next compiler you compile it on
> might not have such a function.
>

are you talking about clrscr() ? Ok I will remove it then.

> You have not allocates space for copying.

Will this work in your opinion -

char *s = "sphere.dat";

or

char s[] = "sphere.dat";

But other wise what is wrong in using strcpy ?

>
> > if(!(read_dat_file(s , obj)))
>
> Anything other than zero is regarded as "true" in C. Therefore the -1
> that you return from read_dat_file will also be regarded as true. The
> following printf statement will execute only when read_dat_file returns
> zero, which it never does.
>
> > printf("unsuccessful");
> > return 0;
>
> > }

So I guess I should return 0 in read_dat_file function in case the
file cannot be read and 1 if the operation is successful.

i corrected the things santosh said and now the program is giving
"unsuccessful" as o/p. I do not understand what is the problem in
reading the file and printing it.

Cell wrote:

>
>> This is not a portable function. The next compiler you compile it on
>> might not have such a function.
>>
>
> are you talking about clrscr() ? Ok I will remove it then.
>
>> You have not allocates space for copying.
>
> Will this work in your opinion -
>
> char *s = "sphere.dat";
>
> or
>
> char s[] = "sphere.dat";

Yes, both will.

> But other wise what is wrong in using strcpy ?

Nothing. But in the statement:

strcpy(s, "sphere.dat");

the string pointed to by the second argument is copied to the address
given as the first argument. But 's' has not been previously
initialised to point to some valid storage, so the strcpy call will
write to some random address, which could lead to anything from silent
memory corruption to a system crash.

You can initialise 's' like this:

s = malloc(strlen("sphere.dat") + 1);

>> > if(!(read_dat_file(s , obj)))
>>
>> Anything other than zero is regarded as "true" in C. Therefore the -1
>> that you return from read_dat_file will also be regarded as true. The
>> following printf statement will execute only when read_dat_file
>> returns zero, which it never does.
>>
>> > printf("unsuccessful");
>> > return 0;
>>
>> > }
>
> So I guess I should return 0 in read_dat_file function in case the
> file cannot be read and 1 if the operation is successful.

That's one possibility. All that C knows about is that zero is false, a
non-zero value is true.