-6

I'm trying to sort through an array of structs and I'm having trouble correctly sorting through the array. I have tried using pointer arithmetic, memcpy and array notation to sort through. Is there a correct way to do it? The result is just the first record copied over all of them.

void bubblesort(struct Record *ptr, int records,
        int (*fcomp)(const void *, const void *))
{
        int swapped;
        int i = 0;
        struct Record *tmp;
        struct Record *tmp1;
        do {
                swapped =0;
                for(i=0;i<records-1;i++){
                        if(fcomp(ptr+i,ptr+i+1)>0){
                                swapped = 1;
                                tmp = ptr+i;
                        /*      tmp1 = ptr+i+1;
                                ptr[i]= *tmp1;
                                ptr[i+1] = *tmp;
                                */
                        //      tmp->seqnum = ptr[i].seqnum;
                        //      tmp->threat = ptr[i].threat;
                        //      tmp->addrs[0] = ptr[i].addrs[0];
                        //      tmp->addrs[1] = ptr[i].addrs[1];
                        //      tmp->ports[0] = ptr[i].ports[0];
                        //      tmp->ports[1] = ptr[i].ports[1];
                        //      strcpy(tmp->dns_name,ptr[i].dns_name);
                                ptr[i].seqnum = ptr[i+1].seqnum;
                                ptr[i].threat = ptr[i+1].threat;
                                ptr[i].addrs[0] = ptr[i+1].addrs[0];
                                ptr[i].ports[0] = ptr[i+1].ports[0];
                                ptr[i].addrs[1] = ptr[i+1].addrs[1];
                                ptr[i].ports[1] = ptr[i+1].ports[1];
                                strcpy(ptr[i].dns_name ,ptr[i+1].dns_name);

                                ptr[i+1].seqnum = tmp->seqnum;
                                ptr[i+1].threat = tmp->threat;
                                ptr[i+1].addrs[0] = tmp->addrs[0];
                                ptr[i+1].ports[0] = tmp->ports[0];
                                ptr[i+1].addrs[1] = tmp->addrs[1];
                                ptr[i+1].ports[1] = tmp->ports[1];
                                strcpy(ptr[i+1].dns_name,tmp->dns_name);


                        }
                }
        }
`
fireguy172
  • 1
  • 3
  • 2
    show us your best guess, we will give you feed back – pm100 Nov 27 '18 at 19:23
  • 2
    We're not mind readers. It is *impossible* to tell you what is wrong with your code unless we can *see it*, along with clearly stated objectives (seems we have most that; you want to bubble-sort a sequence of structures), expected behavior, actual behavior, and whatever debugging work you've already done and the conclusions you've drawn from said-same. Please [edit your question](https://stackoverflow.com/posts/53506719/edit) to include all the aforementioned requested information. – WhozCraig Nov 27 '18 at 19:26
  • Write a compare function, write a swap function, write the bubble function to call the other two as necessary? If you have problems with one of these steps, and after fully debugging it yourself, post a new question about that? – Gem Taylor Nov 27 '18 at 20:02
  • I'd guess that your implementations are all pretty much okay. If you tried different syntax and none of the attempts worked, that's probably a sign that there's an error in another part of the code. Show us a [mcve] and I bet someone will find the problem in no time. – Tim Randall Nov 27 '18 at 20:02
  • How do I upload code on here? @pm100 – fireguy172 Nov 27 '18 at 22:54
  • 1
    You need to learn to use structure assignments; it will make the swapping a lot more convenient. `struct Record tmp = ptr[i]; ptr[i] = ptr[i + 1]; ptr[i + 1] = tmp;` should do the trick. Note that the temporary value is a structure, not a pointer to a structure. – Jonathan Leffler Nov 28 '18 at 00:29
  • It wouldn't let me update the code, using a friends account. That would work but when I do that I get an error about a pointer with a cast. – buddy Nov 28 '18 at 00:32
  • See also [How to implement bubble sort with array of structs?](https://stackoverflow.com/questions/53510259/how-to-implement-bubble-sort-with-array-of-structs) As noted over there, you need to learn how to use structure assignments. – Jonathan Leffler Nov 28 '18 at 00:46

1 Answers1

0

You should use structure assignments because they are radically more compact than repeated assignments for each element of the structure. Like this:

struct Record
{
    int seqnum;
    int threat;
    int addrs[2];
    int ports[2];
    char dns_name[32];
};

void bubblesort(struct Record *ptr, int records,
                int (*fcomp)(const void *, const void *))
{
    int swapped;
    do
    {
        swapped = 0;
        for (int i = 0; i < records - 1; i++)
        {
            if (fcomp(ptr + i, ptr + i + 1) > 0)
            {
                swapped = 1;
                struct Record tmp = ptr[i];
                ptr[i] = ptr[i + 1];
                ptr[i + 1] = tmp;
            }
        }
    } while (swapped != 0);
}

Note that in this code, the temporary value tmp is a structure, not a pointer to a structure.

You also omitted the while (swapped != 0); condition and the trailing } in the code in the question. I've put them in by inference. That code compiles. I've not run it — I've merely reduced the swapping code to 3 lines from 21 or so.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • Thank you that fixed the issue. Could you explain to me why the way i was doing it before caused the code to store the same value in each record struct? – buddy Nov 28 '18 at 03:54
  • Your `tmp` was a pointer to a `struct Record`, and you made it point to `ptr[i]` (you set it to `ptr + i`, aka `&ptr[i]`. But you made no copy of the data; you just point to the memory that holds `ptr[i]`. Then you do a piecewise copy of the data from `ptr[i+1]` over `ptr[i]` (which, coincidentally, means that `tmp` now points at a copy of the data that was in `ptr[i+1]`). _[…continued…]_ – Jonathan Leffler Nov 28 '18 at 03:59
  • _[…continuation…]_ Then you assign the data that `tmp` points to over `ptr[i+1]`, which is a no-op in terms of value changes since `tmp` points at a copy of what was in `ptr[i+1]`. So now you have two adjacent rows containing what was in `ptr[i+1]` and nothing containing what was in `ptr[i]`. This is not what was wanted. – Jonathan Leffler Nov 28 '18 at 03:59
  • Ok thank you I that makes since. I'm still learning how to properly use pointers with structs. – buddy Nov 28 '18 at 04:34