What's wrong with this code?

L

LL

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

struct Student {
int id;
char* name;
};


void getStudents(struct Student*, int);
void printStudents(struct Student*, int);

main() {
int n;
struct Student *s;
printf("Number of students: ");
scanf("%d", &n);
s=(struct Student*)malloc(n*(sizeof(struct Student)));
getStudents(s,n);
printStudents(s,n);
free(s);
exit(0);
}

void getStudents(struct Student *s, int n) {
for (int i=0; i<n; i++) {
printf("Student Name: ");
scanf("%s", s->name);
printf("Student ID: ");
scanf("%d", &s->id);
s++;
}
}


void printStudents(struct Student *s, int n) {
for(int i=0;i<n;i++) {
printf("Student Name: %s", s->name);
printf("ID: %d", s->id);
s++;
}
}
 
I

Ian Collins

LL said:
#include <stdio.h>
#include <stdlib.h>
Are you attempting to write C or C++?
struct Student {
int id;
char* name;
};


void getStudents(struct Student*, int);
void printStudents(struct Student*, int);

main() {
int n;
struct Student *s;
printf("Number of students: ");
scanf("%d", &n);
s=(struct Student*)malloc(n*(sizeof(struct Student)));
getStudents(s,n);
printStudents(s,n);
free(s);
exit(0);
}

void getStudents(struct Student *s, int n) {
for (int i=0; i<n; i++) {
printf("Student Name: ");
scanf("%s", s->name);
printf("Student ID: ");
scanf("%d", &s->id);
s++;
}
}


void printStudents(struct Student *s, int n) {
for(int i=0;i<n;i++) {
printf("Student Name: %s", s->name);
printf("ID: %d", s->id);
s++;
}
}

What's your question? What do you think is wrong?
 
R

Rolf Magnus

LL said:
#include <stdio.h>
#include <stdlib.h>

struct Student {
int id;
char* name;

You really should use strings (std::string) instead of raw character arrays.
};


void getStudents(struct Student*, int);
void printStudents(struct Student*, int);

main() {

Your main() function is missing a return type.
int n;
struct Student *s;
printf("Number of students: ");
scanf("%d", &n);
s=(struct Student*)malloc(n*(sizeof(struct Student)));

Prefer operator new over malloc in C++:

s = new Student[n];

Or even better, use std::vector and don't manually allocate dynamic memory
at all. Especially if you're a beginner, that will make things a lot easier.
getStudents(s,n);
printStudents(s,n);
free(s);

delete [] s; // if you use operator new
exit(0);
}

void getStudents(struct Student *s, int n) {
for (int i=0; i<n; i++) {
printf("Student Name: ");
scanf("%s", s->name);

s->name is an uninitialized pointer. So you're writing to whatever random
memory location it currently happens to point to. You need to let it point
to some memory that can take the character data to be read from stdin.
Unfortunately, you don't know how many characters the user is going to
enter. What you can do is allocate some large enough block of memory before
calling scanf and then limit the number of characters that it reads. This
can be done with the format string. Much simpler, and without that limit,
would be to use std::string and std::cin.
 
R

Rolf Magnus

LL said:
#define maxLen 100
struct Student {
int id;
char name[maxLen];
};
struct Student {
int id;
char* name;
};
scanf("%s", s->name);

That fixes it.

Unless the user types a name that is longer than 99 characters, in which
case you get a buffer overflow, one of the most-exploited types of security
leaks in real-world software.
 
R

Rui Maciel

LL said:
#include <stdio.h>
#include <stdlib.h>

struct Student {
int id;
char* name;
};


void getStudents(struct Student*, int);
void printStudents(struct Student*, int);

main() {
int n;
struct Student *s;
printf("Number of students: ");
scanf("%d", &n);
s=(struct Student*)malloc(n*(sizeof(struct Student)));
getStudents(s,n);
printStudents(s,n);
free(s);
exit(0);
}

<snip />


You defined struct Student and in the main() function you allocate memory for instances of struct Student. Yet, notice that you never allocate memory for char *name, which means that in the getStudent() and printStudent() procedures, when you try to access the name fields, you are attempting to read from memory that, as you never allocated, isn't yours. That's a no no.

Other than that, it appears that you only rely on C language constructs and libraries, not C++. Are you trying to learn C or C++?


Hope this helps,
Rui Maciel
 
L

Leson

Firstly, you do not have to name the members of the structure allocate
memory space to store the string;

Secondly, in your GetStudents function the pointer of the structure should
be an alternative variable, otherwise you will not print the correct
structure, because the structure pointer displacement has occurred, and not
the original location. Similarly printStudents.
 

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,961
Messages
2,570,130
Members
46,689
Latest member
liammiller

Latest Threads

Top