2

I pass in a linked list of size 10 to sort_nodes, alongside an int stating so. However when I use quicksort, my program "stops", and it does not seem to execute the second print nodes function. For some reason, my CLion compiler is giving me an "unable to disassemble function" error so I have been unsuccessful even using the debugger.

int main(void){
    node_ptr *head;
    int num_of_nodes = load_nodes(head); // returns the number of items loaded, in this case it is going to be 10
    print_all_nodes(head); 
    sort_nodes(head,num_of_nodes); 
    print_all_nodes(head); // check that the items have been sorted correctly
}


void insert_node(node_ptr *head_ptr, node_ptr insertion){

    if (*head_ptr != NULL){ // if the header has a pointer
        printf("Inserting new item at head\n");
        insertion->next = *head_ptr;
    }
    else{
        printf("Head is null so assigning head to new pointer\n");
    }
    *head_ptr = insertion;

}

int compare_strings(const void *p1, const void *p2){
    competitor *competitor1, *competitor2;
    node1= (node *) p1;
    node2 = (node *) c2;
    return strcmp(node1->some_string,node2 ->some_string);
}

void delete_linked_list(node_ptr *head_ptr){
    node_ptr current_temp_ptr = *head_ptr;
    node_ptr next_temp_ptr = NULL;

    while (current_temp_ptr != NULL){
        next_temp_ptr = current_temp_ptr->next;
        free(current_temp_ptr);
        current_temp_ptr = next_temp_ptr;
    }
    head_ptr = NULL;
}


void sort_nodes(node_ptr *head_ptr, const int num_of_nodes){
    printf("Num of nodes: %d\n",num_of_nodes); // expected 10 got 10;
    node_ptr nodes[num_of_nodes]; // array used for sorting
    node_ptr temp_ptr = *head_ptr; 
    printf("temp pointer set up\n");
    for (int i=0; i<num_of_nodes; i++){
        temp_ptr = temp_ptr->next; // get next (first) node
        nodes[i] = temp_ptr;
    }
    delete_linked_list(head_ptr);
    printf("Num of nodes: %d",num_of_nodes); //expected 10, got 10
    qsort(&nodes,num_of_nodes, sizeof(node), compare_strings);
    printf("Num of nodes= %d", num_of_nodes); // expected 10, got -2145129072
    for (int i=0; i<num_of_nodes; i++){
        insert_node(head_ptr,nodes[i]);
    }
}

Sorry for any typos I've had to change names to make the problem more general

James Green
  • 125
  • 12
  • 1
    Calling `delete_linked_list(head_ptr)` in `sort_nodes` is unlikely to be end well. You have stored all the node pointers into an array and then you have freed all those pointers by calling the delete function. So the array now contains all invalid pointers. – kaylum Jan 03 '22 at 21:37
  • Yeah this just made me realise I don't want to delete the pointers, and they don't need to be, they're just being re-arranged when I re-insert them. – James Green Jan 03 '22 at 21:55
  • 1
    But you do need to NULL out the `*head_ptr`. Otherwise you will be inserting the nodes into a list that already has those notes. So that's the level of "delete" that you do need to do. – kaylum Jan 03 '22 at 21:57
  • 1
    Don't be sorry about typos — make sure there aren't any. Compile the code that you submit before submitting it, to ensure you are getting the exact results you claim from the code you show. Don't even risk the code being wrong. Yes, you might have to make a copy of the code and edit that copy — it isn't rocket science (or even computer science) to do that. I note that your code is not MCVE ([Minimal, Complete, Verifiable Example](https://stackoverflow.com/help/mcve) — or MRE) or an SSCCE ([Short, Self-Contained, Correct Example](http://sscce.org/)) — the same idea by a different name. – Jonathan Leffler Jan 03 '22 at 22:00
  • 2
    See [Is it a good idea to typedef pointers](https://stackoverflow.com/q/750178/15168) — TL;DR the answer is generally "No", with possible exceptions for function pointers. – Jonathan Leffler Jan 03 '22 at 22:03
  • Do you really have 3 different types node_ptr, notes_ptr, and nodes_ptr? – stark Jan 03 '22 at 22:06
  • I've edited it so there aren't three pointers, that was one of the typos (that I obviously wasn't aware of) that I warned about. I also didn't know how else to condense the code. I get this isn't runnable but I didn't know if it was a good idea to include pretty much all of the code when the problem was just with one area. – James Green Jan 03 '22 at 22:35
  • Also, with the typedef, that's just what my university has taught me and I'm of course drawing from previous projects. Thanks, I will take a look at the typedef thing – James Green Jan 03 '22 at 22:36

1 Answers1

2

At least these issues

Wrong type

p1, p2 are pointers to a nodes_ptr, not a pointer to node. Recall that p1, p2 are pointers to the elements of the array and the array was nodes_ptr nodes[num_of_nodes].

int compare_strings(const void *p1, const void *p2){
  // competitor *competitor1, *competitor2;
  // node1= (node *) p1;
  // node2 = (node *) c2;
  const nodes_ptr *node1 = (const nodes_ptr *) p1;
  const nodes_ptr *node2 = (const nodes_ptr *) p2;
  // return strcmp(node1->some_string,node2 ->some_string);
  return strcmp((*node1)->some_string, (*node2)->some_string);
}

Wrong element size and & not needed

Rather than size to the type of an element sizeof(node) (and use the wrong type as OP did), size to an array element sizeof nodes[0]. Easier to code right.

nodes_ptr nodes[num_of_nodes]; // array used for sorting
....
// Drop  v                     wrong size              
// qsort(&nodes,num_of_nodes, sizeof(node), compare_strings);
qsort(nodes, num_of_nodes, sizeof nodes[0], compare_strings);
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • 1
    Belated Happy New Year! – David C. Rankin Jan 03 '22 at 22:30
  • Could I instead use sizeof(node_ptr)? – James Green Jan 03 '22 at 22:40
  • 2
    @JamesGreen — yes, but using `sizeof(nodes[0])` (with or without the parentheses — that's a separate theological discussion that doesn't need to be resolved here) is safer. It will be correct even if the type of `nodes` changes, whereas using `sizeof(node_ptr)` will cease to be correct (and may produce the wrong answer) if the type of `nodes` changes. – Jonathan Leffler Jan 03 '22 at 23:39
  • @JamesGreen Re: using `sizeof(node_ptr)`: Code originally used `sizeof(node)` and thus the wrong size. `sizeof(node_ptr)` risks the same thing: matching type with object - recall that good code evolves over time. `sizeof nodes[0]` is correct size regardless of type of the array today or next year. – chux - Reinstate Monica Jan 04 '22 at 00:21
  • Could i also ask if you know how to fix my problem with the "compare_strings" function? I've realised that I'm passing an array of pointers to a function that casts them to actual nodes. This has caused a problem of course, and I'm really not confident enough with pointers to understand where I'm going wrong – James Green Jan 04 '22 at 01:01
  • 1
    @JamesGreen Hmm, I thought the posted code did correct the compare issue. You have coded [typedef pointers](https://stackoverflow.com/questions/70571670/qsort-changing-value-of-passed-array-length-integer/70571904?noredirect=1#comment124752377_70571670) making the task extra challenging. Further your post is not a [mcve]. Try compiling what you have posted. Too many unknowns to readily provide a better answer. – chux - Reinstate Monica Jan 04 '22 at 01:08