Thread: malloc problem

How does it not work?

Did you #include <stdlib.h>?

That fixed it? It sounds unlikely, because sizeof(strlen(buf) + 1) is likely to be 2, 4 or 8. If buf has a length longer than the number of bytes in a size_t, you will get a buffer overflow.

> printf(msg);
> if(gets(buf) == NULL) /* check for errors */
The only useful thing to do with code like this is delete it.

Learning C would be a good idea as well.
Like
> lineptr[i] = (char *) malloc(sizeof(strlen(buf) + 1));
When to use strlen() and when to use sizeof()
Using them both together isn't what you want.

> lineptr = (char **) malloc(students);
Read the malloc FAQ
It will tell you how to allocate the right number of objects with the right size.


> The problem is in strcpy() function in store_students().
The problem is ALL your broken malloc calls.

> printf(msg);
> if(gets(buf) == NULL) /* check for errors */
The only useful thing to do with code like this is delete it.

Whats wrong with that?
printf("%s", msg);
Sure.

Learning C would be a good idea as well.

Isnt thats why im here? :/

I fixed all the mallocs.

Code:

char **lineptr;
lineptr = (char **) malloc(sizeof(char *) * students);

Code:

void store_students(char *lineptr[], int students, char *buf, FILE *fp)
{
     int i;
     
     for(i = 0; i < students; i++) {
           fgets(buf, MAXBUF, fp);
           lineptr[i] = (char *) malloc(strlen(buf) + 1);
           strcpy(lineptr[i], buf);
     }
     fseek(fp, 0l, 0);
}

The reason they are broken because i never really needed this type of malloc.

And K&R covers a little malloc, not much.

Originally Posted by Tool

Whats wrong with that?

The use of gets(), which has no way of preventing a buffer overflow.

Originally Posted by Tool

I fixed all the mallocs.

I suggest that you fix it like this:

Code:

char **lineptr;
lineptr = malloc(sizeof(*lineptr) * students);

So, instead of having to make sure that you got the sizeof correct, you get the compiler to make sure of that for you through sizeof(*lineptr).

Originally Posted by Tool

Im making this program for a teacher.

And im preety sure he's not gonna overflow it, thats why i used gets.

I am pretty sure that many people who allowed such mistakes to creep into their code were pretty sure that those mistakes would never be exploited. I am also pretty sure that some of them were shamed on The Daily WTF when their mistakes were discovered, perhaps even by a user. Would you like to be next, pretty please?

> Im making this program for a teacher.
Surely not to pass onto other students!?

> And im preety sure he's not gonna overflow it, thats why i used gets.
And will you remember to do the right thing when you slip effortlessly from academia to the workplace?
Being 'right' is in the approach from the beginning, not what you can get away with for the moment.

If you have a style that has less bugs, you WILL spend less time debugging.

Originally Posted by Tool

And K&R covers a little malloc, not much.

There is not that much to it -- malloc(x) where x is a number of bytes.

Code:

malloc(sizeof(strlen(buf) + 1));

is pretty unique You should look up the return type of a function if you are going to use it.

Using sizeof() on a string is a common mistake tho.

You're right Salem, i'll write the code the best i know and can - simply because then i'll get used to it and avoid errors later on.

Can you have a quick look at my code and tell me what did i miss/do wrong?

Code:

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

#define MAXBUF 100

void error(char *msg, int x); 

void initrand();
int randint();

void menu();
void print_help();

FILE *file_name(char *buf, char *msg, FILE *fp)
{
     char *p;
     printf("%s", msg);
     if(fgets(buf, MAXBUF, stdin) == NULL) /* check for errors */
         error("Greska pri unosu, zavrsavam.\n", 1); 
     if((p = strchr(buf, '\n')) != NULL)
         *p = '\0';
     strcat(buf, ".txt");
     if((fp = fopen(buf, "r")) == NULL) /* open file with student names */
         error("Ne mogu otvoriti navedeni file.\n", 2);
     return fp;
}

int number_of_lines(char *buf, FILE *fp)
{
     int lines;
     
     lines = 0;
     while(fgets(buf, MAXBUF, fp) != NULL)
         lines++;
     
     fseek(fp, 0l, 0);
     return lines;
}

void store_data(char *lineptr[], int students, char *buf, FILE *fp)
{
     int i;
     
     for(i = 0; i < students; i++) {
           if(fgets(buf, MAXBUF, fp) == NULL)
               error("reading failed.\n", 1);
           if((lineptr[i] = (char *) malloc(strlen(buf) + 1)) == NULL)
               error("malloc failed.\n", 2);
           strcpy(lineptr[i], buf);
     }
     fseek(fp, 0l, 0);
}

void print_students(int students, char *lineptr[])
{
     int i;
     
     for(i = 0; i < students; i++)
         printf("%d: %s", i + 1, lineptr[i]);
     printf("\nUkupno ucenika: %d\n", students);
     
}

#define YES 1
#define PROCEED 2
#define QUIT 3
int questioning(char *lineptr[], char *qsts[], int students, int num_qsts)
{
     int c;
     int option();                                 /* get option wether to ask a student */
     int print_questions(int qsts, char *qsts[]);  /* prints questions */
     
     int rand;           /* random number */
     initrand();         /* initialize random seed */
     
     printf("\nZa slucajne brojeve, [ENTER].\n");
     printf("Za kraj,  pritisnite [x]\n\n");
     if((c = option()) == QUIT)
         return 'x';
     else if(c != EOF)
         c = '\n';
     while(c != EOF) {
        if(c != '\n' && c != 'x') { /* only a newline triggers a new rand student to be written to stdout */
            c = getchar();
            continue;
        }
        if(c == 'x')
            return 'x';
        printf("%d: ", rand = randint() % students + 1);
        printf("%s", lineptr[rand - 1]);
        printf("Pitati? 'd' za da, u suprotnom enter.\n");
        switch(option()) { /* ask the student with random questions? */
            case YES:
                 if(print_questions(num_qsts, qsts) == 'x') 
                     c = 0;
                 else
                     return EOF;
                 break;
            case QUIT:
                 c = 'x';
                 break;
            case EOF:
                 return EOF;
            default:
                    break;
        }
     }
     return EOF;
}

int option()
{
     char buf[MAXBUF];
     
     if(fgets(buf, MAXBUF, stdin) == NULL)
        return EOF;
        
     if(buf[0] == 'd' && buf[1] == '\n')
        return YES;
     else if(buf[0] == 'x' && buf[1] == '\n')
        return QUIT;
     else
         return PROCEED;
}

int get_qstnumber(int questions)
{
     int num;
     char buf[MAXBUF];
     
     while(1) {
           if(fgets(buf, MAXBUF, stdin) == NULL)
              return EOF;
           num = atoi(buf);
           if(num > 1 && num < questions) 
               return num;
           else
               printf("\ninterval od 1 do %d: ", questions);
     }
}
    
int print_questions(int questions, char *qsts[])
{
     int num;
     initrand();
     
     printf("Broj pitanja(u bazi %d pitanja): ", questions);
     if((num = get_qstnumber(questions)) == EOF)
         return EOF;
     
     int c, rand;
     while((c = getchar()) != EOF) {
         if(c != '\n' && c != 'x')
             continue;
         if(c == 'x')
             return 'x';
         printf("%d: ", rand = randint() % num + 1);
         printf("%s", qsts[rand - 1]);
     }
     return EOF;
}


#define UNSUPPORTED_VAL 4
void get_choice(char *buf, int *choice)
{
     if(fgets(buf, MAXBUF, stdin) == NULL) {
        *choice = EOF;
        return;
     }
     if(buf[0] == '1' && buf[1] == '\n')
        *choice = '1';
     else if(buf[0] == '2' && buf[1] == '\n')
        *choice = '2';
     else if(buf[0] == '3' && buf[1] == '\n')
        *choice = '3';
     else
         *choice = UNSUPPORTED_VAL;
}
int main(int argc, char *argv[])
{
     char buf[MAXBUF];  /* buffer */
     FILE *fp;          /* students file */
     FILE *predmet;     /* questions file*/
     int students;      /* number of students */
     int questions;     /* number of questions */
     
     fp        = file_name(buf, "Unesite razred: ", fp); /* open file containing student names */
     predmet   = file_name(buf, "Predmet: ", predmet);   /* open file containing questions */
     students  = number_of_lines(buf, fp);               /* count number of students (lines) */
     questions = number_of_lines(buf, predmet);          /* count number of questions (lines) */
     
     char **lineptr;
     char **qstsptr;
     lineptr = (char **) malloc(sizeof(char *) * students);
     qstsptr = (char **) malloc(sizeof(char *) * questions);
     store_data(lineptr, students, buf, fp);       /* store student names into lineptr */
     store_data(qstsptr, questions, buf, predmet); /* store questions into qstsptr */
     
     printf("\n[%d] ucenika nadjeno.\n", students);
     /**********************MAIN MENU******************************************/
     int choice;       /* menu choice */           
     do {
         menu();       /* print menu to stdout */
         get_choice(buf, &choice);
         if(choice == '1')  /* list student names */
                print_students(students, lineptr);
         else if(choice == '2')  /* questioning */
                choice = questioning(lineptr, qstsptr, students, questions);
         else if(choice == '3')  /* print help */
                print_help(); 
     } while(choice != EOF);
     /*************************************************************************/
     return 0;
}

void initrand()
{
    srand((unsigned)(time(0)));
}

int randint()
{
    return rand();
}

void print_help()
{
     printf("-->POMOC<--\n");
     printf("--da bi dodali pristup drugim razredima, potrebno je napraviti sljedece:\n");
     printf("--u lokalnom direktoriju ovog .exea napravite .txt file.\n");
     printf("--renameate ga u odgovarajuce ime, npr 4a.txt ili 4f.txt.\n");
     printf("--u njega unesite imena ucenika tog razreda\n");
     printf("--vrlo je bitno da je po jednom retku samo jedno ime i prezime!\n");
     printf("--isto tako je vrlo bitno da su ucenici poredani kao u imeniku!\n");
     printf("--u suprotnom program nece raditi ocekivano.\n");
     printf("--kada ga kreirate odavde mu napisite ime za otvaranje kako bi ga koristili.\n");
     printf("--vrlo je bitno da fileove sa ekstenzinojim 4c_pitani.txt i ostale ne DIRATE.\n");
     printf("--izbjegavati palatale i ostale hrvatske sumnike.\n\n");
}

void menu()
{
     printf("Glavni meni:\n");
     printf("       1 - lista svih ucenika ovog razreda.\n");
     printf("       2 - ispitivanje.\n");
     printf("       3 - pomoc.\n");
     printf("Izbor: ");
}

void error(char *msg, int x)
{
     printf(msg);
     getchar();
     exit(x);
}