Problem with allocating memory for a char pointer

Hi, this program I wrote is about creating a notebook using linked list as a final requirement. A notebook with a page number, line number and also a text on each line number.

Here is my insert note function where I will enter the Page#, Line#, and the Text and the inputs will just be passed to another functions where the nodes will be inserted.

When calling this function for the first time, there is no problem, the node will be inserted. but on the second call of this function, the program will stop on the statement which allocates memory for variable txt which is a char pointer.

I can't figure out what is the problem with the code. I tried inserting the code "free(txt);" at the bottom of the function before the "return;" statement but it will cause an error even on the first call of this function.

Can anyone help me? Thanks in advance.
Here is my code below:

Code:

---

void InsertNote(){

int pn,ln,i;

char *txt,c;
struct notebook *temp;
temp=note;

txt = (char *) malloc(sizeof(char));
printf("\n\nINSERT NOTE");
printf("\nWhat Page Number? ");
scanf("%d",&pn);
printf("What Line Number? ");
scanf("%d",&ln);

fflush(stdin);
printf("Enter Your Note: ");
gets(txt);

if(temp==NULL){

AddFirst(txt,pn,ln);

}
else{

while(temp!=NULL){

if(temp->pagenum <= pn){

if(temp->linenum < ln){

c++;

}

}

temp=temp->next;

}

if(c==0){

AddFirst(txt,pn,ln);

}
else if(c<count()){

AddAfter(txt,pn,ln,++c);

}
else{

Append(txt,pn,ln);

}

}
//free(txt);
return;

}

---

Code:

---

    txt = (char *)malloc(sizeof(char));
   
    gets(txt)

---

The malloc() call allocates memory for exactly one char. The gets() call reads an arbitrary number of characters, and writes a value with character zero at the end. If that arbitrary number of characters exceeds 0, the buffer length of one is being exceeded. Hence the program overwrites a random area of memory. (Technically this is a form of undefined behaviour).

Other problems;

1) fflush(stdin) has undefined behaviour. Don't do it.
2) Don't use gets(), as it is unsafe (able to read any number of characters, regardless of the length of the buffer provided). Use fgets() instead.
3) No need to place the (char *) type conversion on the malloc() call.

1. Why do you cast the result of malloc? In C we normally write it like this

char *txt = malloc(sizeof(char));

2. The above means to allocate 1 character. Is that big enough for your input??

3. Why do you flush stdin? It's not needed and it may produce unpredictable results.

4. Why do you use gets? There are better alternatives like fgets if you want your code to work reliably

5. You are incrementing c but you never assign it an initial value. Same with the statement "if c==0". This means your code as it stands will produce unpredictable results.

6. You call your AddFirst function in two different places. You should only call it in one situation. If you assign an initial value to c, then I think you will see that these two situations are actually the same and you should reduce them to one case for the sake of logical clarity.

Thanks guys.. the above problem is solved. and i found again another problem. i have two function the AddFirst() and Append() both will insert a node. there is no problem with the AddFirst function but if i will add another node which will call the Append() function the error will occur on reading the statement "strcpy(temp->line,str);". and again i can't figure out what is the cause of the error.

Code:

---

void Append(char *str, int p, int l){
    struct notebook *temp,*right;
    temp=(struct notebook *)malloc(sizeof(struct notebook));
    //temp=malloc(sizeof(struct notebook));
    strcpy(temp->line,str);
    temp->pagenum=p;
    temp->linenum=l;
   
    right=(struct notebook *)note;
    while(right->next != NULL){
      right=right->next;
    }
   
    right->next = temp;
    right=temp;
    right->next=NULL;
   
    //return;
}

void AddFirst(char *str, int p, int l){
    struct notebook *temp;
    temp=(struct notebook *)malloc(sizeof(struct notebook));
   
    strcpy(temp->line,str);
    temp->pagenum=p;
    temp->linenum=l;
   
    if(note == NULL){
      note=temp;
      note->next=0;
    }
    else{
      temp->next=note;
      note=temp;
    }
    //return;
}

---

Code:

---

void Append(char *str, int p, int l){
    struct notebook *temp,*right;
    temp=(struct notebook *)malloc(sizeof(struct notebook));
    //temp=malloc(sizeof(struct notebook));
    strcpy(temp->line,str);

---

How is your "struct notebook" defined?

Don't cast the return value of malloc!

Bye, Andreas

Here is my struct:

Code:

---

struct notebook{
    char *line;
    int pagenum, linenum;
    struct notebook *next;
}*note;

---

oh i'm using wxDev-C++ compiler version number build 7.4.2.569
also using .c extension

Which means that you need
temp->line = malloc(strlen(str)+1);

before you do this
strcpy(temp->line,str);

Quote:

---

Originally Posted by jmyrrah

there is no problem with the AddFirst function

---

Wrong, you have the same problem as in Append():

Code:

---

void AddFirst(char *str, int p, int l){
    struct notebook *temp;
    temp=(struct notebook *)malloc(sizeof(struct notebook));
   
    strcpy(temp->line,str);    // temp->line doesn't point to valid memory

---

Bye, Andreas

but why is it that when i call the function display, it will display page number, line number, and the text.

sorry i'm was wrong.. because it already works when i put the temp->line = malloc(strlen(str)+1); before strcpy(temp->line,str); on the AddFirst() function

thanks you so much guys..

i'll post again if there's another problem i will encounter..

Quote:

---

Originally Posted by jmyrrah

but why is it that when i call the function display, it will display page number, line number, and the text.

---

Code with undefined behaviour is like that.

When code has undefined behaviour, there are no constraints on what happens as a result. In other words, any result is permitted.

One of the infinite number of permitted results is appearing to behave as you expect in some circumstances (eg within the function display) and crashing in others (eg when calling strcpy()). Both involve undefined behaviour if you haven't done a suitable malloc() call, but the behaviour of both functions is different.