What's wrong with this code?

Discussion in 'C++' started by LL, Jan 11, 2009.

  1. LL

    LL Guest

    #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++;
    }
    }
     
    LL, Jan 11, 2009
    #1
    1. Advertising

  2. LL

    Ian Collins Guest

    LL wrote:
    > #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?

    --
    Ian Collins
     
    Ian Collins, Jan 11, 2009
    #2
    1. Advertising

  3. LL

    LL Guest

    On Sun, 11 Jan 2009 20:08:43 +1100, Ian Collins <>
    wrote:

    > LL wrote:
    >> #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?
    >

    When I compile and run the program it crashes.
     
    LL, Jan 11, 2009
    #3
  4. LL

    news Guest

    #define maxLen 100
    struct Student {
    int id;
    char name[maxLen];
    };
    > struct Student {
    > int id;
    > char* name;
    > };
    > scanf("%s", s->name);
     
    news, Jan 11, 2009
    #4
  5. LL

    LL Guest

    On Sun, 11 Jan 2009 20:26:39 +1100, news <> wrote:

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

    >


    That fixes it.
     
    LL, Jan 11, 2009
    #5
  6. LL

    Rolf Magnus Guest

    LL wrote:

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

    > 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++;
    > }
    > }
     
    Rolf Magnus, Jan 11, 2009
    #6
  7. LL

    Rolf Magnus Guest

    LL wrote:

    > On Sun, 11 Jan 2009 20:26:39 +1100, news <> wrote:
    >
    >> #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.
     
    Rolf Magnus, Jan 11, 2009
    #7
  8. LL

    Rui Maciel Guest

    LL wrote:

    > #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
     
    Rui Maciel, Jan 11, 2009
    #8
  9. LL

    Leson Guest

    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.

    "LL" <> ??????:eek:p.unk4ibgccqb7ql@user1-pc...
    > #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++;
    > }
    > }
     
    Leson, Jan 12, 2009
    #9
    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. walala
    Replies:
    3
    Views:
    2,194
    Ralf Hildebrandt
    Sep 10, 2003
  2. willem oosthuizen

    What is wrong with the following code?

    willem oosthuizen, Oct 10, 2003, in forum: VHDL
    Replies:
    9
    Views:
    1,281
  3. Matthew
    Replies:
    7
    Views:
    676
    Priscilla Walmsley
    Jan 7, 2005
  4. David. E. Goble
    Replies:
    9
    Views:
    480
    David. E. Goble
    Feb 2, 2005
  5. kiran
    Replies:
    12
    Views:
    1,129
    Scott Sauyet
    Dec 7, 2011
Loading...

Share This Page