-1

I have this code that creates a linked list but I get a segmentation fault error. Any solutions?

    struct node *temp = (struct node*)malloc(sizeof(struct node));
    char *a = title_parser(bytes, temp);
    head = temp;

    temp = (struct node*)malloc(sizeof(struct node));
    title_parser(a, temp);

    puts(a); // Is NULL

    struct node* temp1 = head;
    while(temp1->link != NULL)
    {
            temp1 = temp1->link;
    }
    temp1->link = temp;

EDIT: After the program traverses the function title_parser, memory is allocated for temp again but when the second time title_parser is called, this is when segmentation fault occurs.

This is my code for title_parser

char* title_parser(char *bytes, struct node *temp){

char *ptr = strstr(bytes, "<title>");


if (ptr) {

    /* Skip over the found string */
    ptr += 7;
    char *ptr2 = strstr(ptr, "</title>");
    if (ptr2) {
        char* output = malloc(ptr2 - ptr + 1);
        memcpy(output, ptr, ptr2 - ptr);

        output[ptr2 - ptr] = 0;

        if(strcmp(output,"TechCrunch")!=0 && strcmp(output,"VentureBeat")!=0){
            temp->title = output;
            temp->link = NULL;
            puts(temp->title);
            free(output);
            char *load = pubdate_parser(ptr2, temp);
            return load;
        }
        else{
            title_parser(ptr2, temp);
        }
    }
}

}

EDIT: Another problem is that when title_parser returns load, I print a in main and the output is NULL.

Miguel Lorenzo Panagsagan
  • 29
  • 1
  • 5
  • How do you know the segmentation fault is caused by the `malloc` line? – Yu Hao May 20 '15 at 02:48
  • You know that you are allocating memory twice for the same pointer? – Spaceghost May 20 '15 at 02:53
  • Use a debugger to find out exactly which line of code is causing the seg fault. – kaylum May 20 '15 at 03:13
  • you need to show more code e.g. title_parser and how the 'node' looks like, it doesn't look like you initialize 'link' from the code you show and you still check if its null – AndersK May 20 '15 at 03:13
  • What is in "bytes"? This code is really hard to read because of the `temp` being used in two places, and `temp1` as well. Also the while loop will never terminate since you're not changing `temp1->link`. – Hut8 May 20 '15 at 03:13
  • Post code that reproduces the problem; not for you, *for us*. The posted code offers no evidence that `temp`'s members are initialized. If you think whatever `title_parser` does to `temp` isn't relevant, think again. If `temp`'s `link` member isn't initialized in that function to NULL, this code invokes undefined behavior. – WhozCraig May 20 '15 at 03:28
  • 1
    `free(output);` That doesn't belong in your function. You just saved a pointer to that memory: `temp->title = output;`, and I'm going to guess you don't want `temp->title` to leave this function with a dangling value. – WhozCraig May 20 '15 at 03:45
  • "Another problem is that when title_parser returns load, I print a in main and the output is NULL." - `load` is synonymous with the return value of `pubdate_parser`, which once again, *we don't have*. Pretend for a moment you don't have the code needed to reproduce your problem; now stop pretending, because that is *exactly* the position *we* are in. Once more *Post code that reproduces the problem; not just for you: for us.* It is called an [MCVE](https://stackoverflow.com/help/mcve). If we can copy/paste what you post and reproduce the problem, you did it right. Otherwise... – WhozCraig May 20 '15 at 04:41
  • [Please don't cast the return value of `malloc()` in C](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). – unwind May 20 '15 at 07:04

2 Answers2

0

In my opinion,this fault isn't caused by malloc line,you can check your function title_parser(bytes, temp)，you can note it,and run your program again. this is my test code:

    #include<stdio.h>
    #include<stdlib.h>
    struct node{
        int val;
        struct node *next;
    };

    void main()
    {
       struct node *tmp=(struct node*)malloc(sizeof(struct node));
       tmp->val=1;
       struct node *head=tmp;
       tmp=(struct node*)malloc(sizeof(struct node));
       tmp->val=2;
       printf("%d\n",tmp->val);
    }

The output is 2,so segmentfault didn't appear as expected.You can have a try!

user3039955
  • 9
  • 1
  • 2
0

In 

if(strcmp(output,"TechCrunch")!=0 && strcmp(output,"VentureBeat")!=0)
{
  temp->title = output;
  temp->link = NULL;
  puts(temp->title);
  free(output);                              // error
  char *load = pubdate_parser(ptr2, temp);
  return load;
}

You are freeing output after assigning title to point to the malloced block this will give you problems when you later try to access the memory.

Also it is good to use more describing variable names than temp and temp1 by reusing variables and using generic names you make the code unnecessary hard to read.

You could also use a variable where it counts like when you calculate the length of the string to make it a little bit clearer as well as catch any error if for some reason the string has the wrong format

so instead of

    char* output = malloc(ptr2 - ptr + 1);
    memcpy(output, ptr, ptr2 - ptr);

    output[ptr2 - ptr] = 0;

you could do

    int titleLength = ptr2 - ptr1;
    if ( titleLength > 0 )
    {
      output = malloc(titleLength + 1);
      strncpy(output, ptr, titleLength);
      output[titleLength] = '\0';
    }
    else { some error message }

It is a bit hard to see what is expected to be returned from the title_parser function, the name of the function implies that it returns the title but it does not seem the case.

AndersK
  • 35,813
  • 6
  • 60
  • 86