pointer to struct

  • Thread starter =?iso-8859-1?B?cG92b2Hn428=?=
  • Start date
?

=?iso-8859-1?B?cG92b2Hn428=?=

can´t understand why the program above not works.
thanks all


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

#define SIZE 3

struct peoples
{
int code;
char name[20];
float value;
};



int main()
{
int a;
struct peoples *people;
people=(struct peoples *)malloc(SIZE*sizeof(struct peoples));
for(a=0;a<SIZE;a++)
{
people->code=a;
scanf("%s",people->name);
scanf("%f",people->value);
people++;
}
for(a=0;a<SIZE;a++)
{
printf("%d",people->code);
printf("%s",people->name);
printf("%f",people->value);
people++;
}
}
 
L

ls

You only have on struct and one pointer. Why are you incrementing the
pointer? Why are you looping? Maybe you need an array of structs 3
(SIZE) pointers and need to loop through the array to allocate structs,
loop to assign values, then loop to acces the values.
 
B

Ben Pfaff

povoação said:
int main()

I'd recommend using the prototype form:
int main(void)
{
int a;
struct peoples *people;

This is very creative naming, to give the pointer to the
beginning of an array a relatively singular name, and the
structure type that the array contains a relatively plural name.
Most programmers would probably do it differently, e.g.
struct person *people;
people=(struct peoples *)malloc(SIZE*sizeof(struct peoples));

I don't recommend casting the return value of malloc():

* The cast is not required in ANSI C.

* Casting its return value can mask a failure to #include
<stdlib.h>, which leads to undefined behavior.

* If you cast to the wrong type by accident, odd failures can
result.

In unusual circumstances it may make sense to cast the return value of
malloc(). P. J. Plauger, for example, has good reasons to want his
code to compile as both C and C++, and C++ requires the cast, as he
explained in article <[email protected]>.
However, Plauger's case is rare indeed. Most programmers should write
their code as either C or C++, not in the intersection of the two.

When calling malloc(), I recommend using the sizeof operator on
the object you are allocating, not on the type. For instance,
*don't* write this:

int *x = malloc (128 * sizeof (int)); /* Don't do this! */

Instead, write it this way:

int *x = malloc (128 * sizeof *x);

There's a few reasons to do it this way:

* If you ever change the type that `x' points to, it's not
necessary to change the malloc() call as well.

This is more of a problem in a large program, but it's still
convenient in a small one.

* Taking the size of an object makes writing the statement
less error-prone. You can verify that the sizeof syntax is
correct without having to look at the declaration.
for(a=0;a<SIZE;a++)
{
people->code=a;
scanf("%s",people->name);

You need to specify a maximum length, or this can lead to a
buffer overflow.
scanf("%f",people->value);

You forgot to take the address: &people->value.
people++;
}
for(a=0;a<SIZE;a++)
{
printf("%d",people->code);
printf("%s",people->name);
printf("%f",people->value);
people++;
}

This isn't going to work, because in your first loop you already
advanced "people" past the records you're trying to print. I'd
suggest dropping the "people++" from both loops and using array
index notation instead for each member reference,
e.g. people[a].code.
 
M

matevzb

povoação said:
can´t understand why the program above not works.
For many reasons.
people=(struct peoples *)malloc(SIZE*sizeof(struct peoples));
for(a=0;a<SIZE;a++)
{
people->code=a;
scanf("%s",people->name);
scanf("%f",people->value);
people++;
}
You're incrementing people (pointer to struct peoples), which after the
first for loop points to where? You'd be better of with:
people[a].code = a;
Secondly, scanf() arguments should be pointers so use:
scanf ("%f", &people[a].value);
For char arrays, you can use either people[a].name or
&people[a].name[0]. Some prefer the second version.
Thirdly, you're not checking for scanf() return values. If someone
doesn't enter a float when you expect it, your program will misbehave.
Also, your people[a].name is limited to 20 characters, it won't work if
someone enters more than that - check the scanf() man page.
}
for(a=0;a<SIZE;a++)
{
printf("%d",people->code);
printf("%s",people->name);
printf("%f",people->value);
people++;
}
Again, don't increment people, use people[a]. Outputting a newline now
and then may also be a good thing.
 
?

=?iso-8859-1?B?cG92b2Hn428=?=

I modified to people[a] instead pointer and works well, but I´m
learning pointers and I can´t understand cause was not working with
pointer.
I corrected scanf, I´d miss &.
I try modify also the line people=malloc(SIZE * sizeof *people); and
i got error at compile time invalid conversion from void* to peoples*.

thanks



povoação said:
can´t understand why the program above not works.For many reasons.
people=(struct peoples *)malloc(SIZE*sizeof(struct peoples));
for(a=0;a<SIZE;a++)
{
people->code=a;
scanf("%s",people->name);
scanf("%f",people->value);
people++;
}You're incrementing people (pointer to struct peoples), which after the
first for loop points to where? You'd be better of with:
people[a].code = a;
Secondly, scanf() arguments should be pointers so use:
scanf ("%f", &people[a].value);
For char arrays, you can use either people[a].name or
&people[a].name[0]. Some prefer the second version.
Thirdly, you're not checking for scanf() return values. If someone
doesn't enter a float when you expect it, your program will misbehave.
Also, your people[a].name is limited to 20 characters, it won't work if
someone enters more than that - check the scanf() man page.
}
for(a=0;a<SIZE;a++)
{
printf("%d",people->code);
printf("%s",people->name);
printf("%f",people->value);
people++;
}Again, don't increment people, use people[a]. Outputting a newline now
and then may also be a good thing.
 
B

Ben Pfaff

povoação said:
I try modify also the line people=malloc(SIZE * sizeof *people); and
i got error at compile time invalid conversion from void* to peoples*.

It sounds like you're using a C++ compiler.
 
M

matevzb

povoação said:
I modified to people[a] instead pointer and works well, but I´m
learning pointers and I can´t understand cause was not working with
pointer.
Well, think of it this way. You are allocating memory that will hold
three structures. malloc() returns the pointer to this memory and you
assign it to variable people. When you increment people (people++),
people will then point to the next element and you no longer have a
pointer to the allocated block:
| 0 | 1 | 2 | | 0 | 1 | 2 |
^ ^
people people

But in order to print out the values in the second loop, you do need
the original pointer, however people is not pointing there anymore.
Either access the allocated memory as an array (people[a]), or remember
the original pointer and after before the second loop, set the people
to point there.
I try modify also the line people=malloc(SIZE * sizeof *people); and
i got error at compile time invalid conversion from void* to peoples*.

thanks
I cannot comment on the sizeof usage, as my perception differs a lot
from what Ben said. I never use sizeof without ()s and I never use it
on variables, but types insetad. The reason for that is the following:
char buf[10];
...
some_function (sizeof (buf));
If in future the buf type is changed from an array to a char *,
sizeof(buf) will get a _very_ different meaning. In small projects, the
bug will be easy to find, however once you have > 100,000 lines of code
and the crash only happens on a customer's machine to which you don't
have access (and, of course, the bug is not reproducible in the lab),
you could spend days before the problem is found. Been there, done
that. So, where Ben said "Don't do this!", I always do =)
 
?

=?iso-8859-1?B?cG92b2Hn428=?=

It´s true. I saved .c and now compiles. I´m using Dev-C++ now.
Can help me to fix the program to work with pointers notation?
 
B

Ben Pfaff

povoação said:
Can help me to fix the program to work with pointers notation?

If a problem is easily solved using array notation, then there is
no need to solve it another way.
 
F

Fred Kleinschmidt

povoação said:
can´t understand why the program above not works.For many reasons.
people=(struct peoples *)malloc(SIZE*sizeof(struct peoples));
for(a=0;a<SIZE;a++)
{
people->code=a;
scanf("%s",people->name);
scanf("%f",people->value);
people++;
}You're incrementing people (pointer to struct peoples), which after
the
first for loop points to where? You'd be better of with:
people[a].code = a;
Secondly, scanf() arguments should be pointers so use:
scanf ("%f", &people[a].value);
For char arrays, you can use either people[a].name or
&people[a].name[0]. Some prefer the second version.
Thirdly, you're not checking for scanf() return values. If someone
doesn't enter a float when you expect it, your program will misbehave.
Also, your people[a].name is limited to 20 characters, it won't work if
someone enters more than that - check the scanf() man page.
}
for(a=0;a<SIZE;a++)
{
printf("%d",people->code);
printf("%s",people->name);
printf("%f",people->value);
people++;
}Again, don't increment people, use people[a]. Outputting a newline
now
and then may also be a good thing.

povoação said:
I modified to people[a] instead pointer and works well, but I´m
learning pointers and I can´t understand cause was not working with
pointer.

in your first loop, you included:
for(a=0;a<SIZE;a++) {

people++;
}

So, where does "people" now point?

Then your second loop:

for(a=0;a<SIZE;a++) {
printf("%d",people->code);
people++;
}

Just because you began a new loop, this does not
mean the "people" magically again points to the
original place.

If you insist on using pointers, try:

struct peoples *p = people;
for(a=0;a<SIZE;a++) {
scanf("%s",p->name);
scanf("%f",&p->value);
p++;
}
p = people; /* resets to point to origin of allcated structs */
for(a=0;a<SIZE;a++) {
...
p++;
}

(still contains problems pointed out by others)

Fred L. Kleinschmidt
Boeing Associate Technical Fellow
Technical Architect, Software Reuse Project
 
K

Keith Thompson

Ben Pfaff said:
If a problem is easily solved using array notation, then there is
no need to solve it another way.

Unless the actual problem is learning to use pointer notation.
 
?

=?iso-8859-1?B?cG92b2Hn428=?=

thanks all. Now works with pointers. Thanks for the tips too.
 

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,769
Messages
2,569,579
Members
45,053
Latest member
BrodieSola

Latest Threads

Top