i have some problems sorting a struct field and cannot find the answer why this is happening... The Error message when i run the program says "Memory access error (German: Speicherzugriffsfehler (Speicherabzug geschrieben)). The error only comes whenever i try the proceed one of the following lines in the bubblesort:

x= *mannschaften[i];
*mannschaften[i]=*mannschaften[i 1];
*mannschaften[i 1]=x;
vertauscht=1;

I have the feeling that i might have to save memory with malloc() or calloc() at some point, but i don't know how to use this,yet...

Thank you for your help! Best regards kazimir

You can see the the entire code below:

#include <stdio.h>
#include <string.h>

typedef struct
{
    char* nation;
    int goals_scored;
    int goals_conceded;
} Team;

typedef struct
{
    Team* t1;
    Team* t2;
    int goals_t1;
    int goals_t2;
}Spiel;

void ergebnis_eintragen(Spiel s)
{
s.t1->goals_scored = s.goals_t1;
s.t2->goals_scored = s.goals_t2;
s.t1->goals_conceded = s.goals_t2;
s.t2->goals_conceded = s.goals_t1;
}

void ausgabe (Team* mannschaften[8])
{
    Team x;
    int m=8;
    int vertauscht;

    do
    {
        vertauscht=0;
        for (int i=0;i<8;i  )
        {
            if (mannschaften[i]->goals_scored<mannschaften[i 1]->goals_scored)
            {
                    x= *mannschaften[i];
                    *mannschaften[i]=*mannschaften[i 1];
                    *mannschaften[i 1]=x;
                    vertauscht=1;
            }

            else if (mannschaften[i]->goals_scored==mannschaften[i 1]->goals_scored||mannschaften[i]->goals_conceded>mannschaften[i 1]->goals_conceded)
            {
                    x = *mannschaften[i];
                    *mannschaften[i]=*mannschaften[i 1];
                    *mannschaften[i 1]=x;
                    vertauscht=1;
            }
        }
        m-=1;
    }
    while (m>1 && vertauscht);

    for (int i=0;i<8;i  )
        printf("\nNation: %s\nTore: %d\nGegentore: %d\n",mannschaften[i]->nation,mannschaften[i]->goals_scored,mannschaften[i]->goals_conceded);
}

int main (void)
{
    Team t1={"Kroatien",0,0};
    Team t2={"Brasilien",0,0};
    Team t3={"Niederlande",0,0};
    Team t4={"Argentinien",0,0};
    Team t5={"Marokko",0,0};
    Team t6={"Portugal",0,0};
    Team t7={"England",0,0};
    Team t8={"Frankreich",0,0};

    Spiel s1={&t1,&t2,5,3};
    Spiel s2={&t3,&t4,5,6};
    Spiel s3={&t5,&t6,1,0};
    Spiel s4={&t7,&t8,1,2};
    Spiel s5={&t4,&t1,3,0};
    Spiel s6={&t8,&t5,2,0};
    Spiel s7={&t1,&t5,2,1};
    Spiel s8={&t4,&t8,7,5};

    Team* ptr_team[8];
    ptr_team[0] = &t1;
    ptr_team[1] = &t2;
    ptr_team[2] = &t3;
    ptr_team[3] = &t4;
    ptr_team[4] = &t5;
    ptr_team[5] = &t6;
    ptr_team[6] = &t7;
    ptr_team[7] = &t8;
    
    ergebnis_eintragen(s1);
    ergebnis_eintragen(s2);
    ergebnis_eintragen(s3);
    ergebnis_eintragen(s4);
    ergebnis_eintragen(s5);
    ergebnis_eintragen(s6);
    ergebnis_eintragen(s7);
    ergebnis_eintragen(s8);
    
    ausgabe(&ptr_team[0]);
}

I already tried looking for a simular question, but couldn't find anything...

CodePudding user response：

In the for loop

    for (int i=0;i<8;i  )
    {
        if (mannschaften[i]->goals_scored<mannschaften[i 1]->goals_scored)

when i is equal to 7 then expressions like this

mannschaften[i 1]->goals_scored)

access memory outside the array ptr_team defined in main.

You could write your loop like for example

    for (int i=1;i<8;i  )
    {
        if (mannschaften[i-1]->goals_scored<mannschaften[i]->goals_scored)
        //...

Also it seems in this if statement

else if (mannschaften[i]->goals_scored==mannschaften[i 1]->goals_scored||mannschaften[i]->goals_conceded>mannschaften[i 1]->goals_conceded)

you mean the logical operator && instead of the logical operator ||

else if (mannschaften[i]->goals_scored==mannschaften[i 1]->goals_scored && mannschaften[i]->goals_conceded>mannschaften[i 1]->goals_conceded)

Using the variable m as is in the do-while loop does not make a great sense. It is enough to use the variable vertauscht

do
{
    //...
} while ( vertauscht);

And it is a bad idea to use magic numbers like 8.

You could declare your function like

void ausgabe (Team* mannschaften[], size_t n );

and call it at least like

ausgabe( ptr_team, 8 );

Then within the function you should use the variable n instead of the magic number 8 as for example

for ( size_t i = 1;i < n;i   )

Otherwise you will need to change your function each time when the size of the passed array will be changed.

You should always write a more general function.

Pay attention to that your program will look much better and will be more readable if you will use English words for identifiers.

CodePudding user response：

Use a debugger or valgrind:

==139555== Use of uninitialised value of size 8
==139555==    at 0x1091FB: ausgabe (demo.c:38)
==139555==    by 0x1098C8: main (demo.c:101)

That's because you are accessing outside of the bounds of the array in the last iteration:

for (int i=0;i<8;i  )
{
    if (mannschaften[i]->goals_scored<mannschaften[i 1]->goals_scored)
----------------------------------------------------^ you access mannschaften[8]

Switch to:

for (int i=0;i<7;i  )

CodePudding user response：

Thank you both very much, problem solved! :)