3

Update *

I have now tried to return something from the function and still the .exe crashes! I am quite new to c so sorry if I am been a bit thick at not spotting why.

struct packet* addRecord(int *rCount, struct packet *records){
int valid = 0;  //used to indicated valid input
int length = 0; //used to store the string lengths
int i = 0;    //used in the for loops
char dataTest[51];     //temporary storage of input to be checked before adding to records




do{
    puts("What is the source of this packet?: ");
    if(scanf(" %c", &records[*rCount].source) == 1){  //if correct insert the record at the index
        valid=1;                                //determined by rCount(the current record count passed to addRecord
    }
    else{
        valid = 0;
        getchar();
        puts("\nNot a valid input");
    }

}while(valid!=1);

do{
    puts("What is the destination of this packet?: ");
    if(scanf(" %c", &records[*rCount].destination) == 1)
    {
        valid = 1;
    }
    else
    {
        valid = 1;
        getchar();
        puts("\nNot a valid input");
    }
   }
   while(valid!=1);
   records = realloc(records,(*rCount+1)*sizeof(struct packet));
   return records;

}

So I have got this code to work, but when I enter a value for &records[*rCount].source, the .exe crashes. I have been looking at this code for an hour now and cannot find the broken link, but I feel like it's something simple.

Here is the little bit of code that I feel like is not working properly.

Also can someone please explain what == 1 means in the if statement, I've kinda just hacked this code together. Thanks

do{
        puts("What is the source of this packet?: ");
        if(scanf("%i", &records[*rCount].source) == 1){  //if correct insert the record at the index
            valid=1;                                //determined by rCount(the current record count passed to addRecord
        }
        else{
            valid = 0;
            getchar();
            puts("\nNot a valid input");
        }

    }while(valid!=1);

Full code:

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

struct packet{ // declare structure for packet creation
        int source;
        int destination;
        int type;
        int port;
        char data[51];
    };

//function prototypes
void listRecords(int, struct packet*);
struct packet* addRecord(int*, struct packet*);
void save(int, struct packet*);
struct packet* open(int*, struct packet*);

int main ()
{
    int recordCount = 0;
    char choice;
    struct packet *records;
    struct packet *temp;

    do {
                printf("\nWhat would you like to do?\n");

                printf("\t1) Add a packet.\n");                 //---------------------//
                printf("\t2) List all packets.\n");             //---------------------//
                printf("\t3) Save packets.\n");                 //---------MENU--------//
                printf("\t4) Clear all packets.\n");            //---------------------//
                printf("\t5) Quit the programme.\n");           //---------------------//

                scanf("%i", &choice); // scan user input and put the entry into variable "choice"
                if(choice == '/n')
                    scanf("%i", &choice);



                switch(choice)
                {
                    case 1: system("cls");
                            records = addRecord(&recordCount, records);
                            break;
                    case 2: system("cls");
                            break;
                    case 3: system("cls");
                            break;
                    case 4: system("cls");
                            break;
                    default: system("cls");
                             printf("%i was not a valid option\n", choice);
                             break;
                }

            }
    while (choice != 5);
    return 0;
}

struct packet* addRecord(int *rCount, struct packet *records){
    int valid = 0;  //used to indicated valid input
    int length = 0; //used to store the string lengths
    int i = 0;    //used in the for loops
    char dataTest[51];     //temporary storage of input to be checked before adding to records




    do{
        puts("What is the source of this packet?: ");
        if(scanf("%i", &records[*rCount].source) == 1){  //if correct insert the record at the index
            valid=1;                                //determined by rCount(the current record count passed to addRecord
        }
        else{
            valid = 0;
            getchar();
            puts("\nNot a valid input");
        }

    }while(valid!=1);

    do{
        puts("What is the destination of this packet?: ");
        if(scanf("%i", &records[*rCount].destination == 1))
        {
            valid = 1;
        }
        else
        {
            valid = 1;
            getchar();
            puts("\nNot a valid input");
        }
       }
       while(valid!=1);
}
Wouter J
  • 41,455
  • 15
  • 107
  • 112
user3103598
  • 185
  • 1
  • 1
  • 10

3 Answers3

2

Change

 if(scanf("%i", &records[*rCount].destination == 1))

to

 if(scanf("%d", &records[*rCount].destination) == 1)  

Also change %i to %d and char choice; to int choice; Another problem is you are returning nothing from your function which has pointer to struct packet return type .

After some changes that I made the compiling code is:

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

struct packet{ // declare structure for packet creation
        int source;
        int destination;
        int type;
        int port;
        char data[51];
    };

//function prototypes
void listRecords(int, struct packet*);
void addRecord(int*, struct packet*);
void save(int, struct packet*);
struct packet* open(int*, struct packet*);

int main (void)
{
    int recordCount = 0;
    int choice;
    struct packet *records;
    //struct packet *temp;

    do {
                printf("\nWhat would you like to do?\n");

                printf("\t1) Add a packet.\n");                 //---------------------//
                printf("\t2) List all packets.\n");             //---------------------//
                printf("\t3) Save packets.\n");                 //---------MENU--------//
                printf("\t4) Clear all packets.\n");            //---------------------//
                printf("\t5) Quit the programme.\n");           //---------------------//

                scanf("%d", &choice); // scan user input and put the entry into variable "choice"
                if(choice == '\n')
                    scanf("%d", &choice);



                switch(choice)
                {
                    case 1: system("cls");
                             addRecord(&recordCount, records);
                             break;
                    case 2: system("cls");
                             break;
                    case 3: system("cls");
                             break;
                    case 4: system("cls");
                             break;
                    default: system("cls");
                             printf("%d was not a valid option\n", choice);
                             break;
                }

            }
    while (choice != 5);
    return 0;
}

void addRecord(int *rCount, struct packet *records){
    int valid = 0;  //used to indicated valid input
    //int length = 0; //used to store the string lengths
    //int i = 0;    //used in the for loops
    //char dataTest[51];     //temporary storage of input to be checked before adding to records




    do{
        puts("What is the source of this packet?: ");
        if(scanf("%d", &records[*rCount].source) == 1){  //if correct insert the record at the index
            valid=1;                                //determined by rCount(the current     record count passed to addRecord
        }
        else{
            valid = 0;
            getchar();
            puts("\nNot a valid input");
        }

    }while(valid!=1);

    do{
        puts("What is the destination of this packet?: ");
        if(scanf("%d", &records[*rCount].destination) == 1)
        {
            valid = 1;
        }
        else
        {
            valid = 1;
            getchar();
            puts("\nNot a valid input");
        }
    }
       while(valid!=1);
}
haccks
  • 104,019
  • 25
  • 176
  • 264
  • this is still crashing although that may help me at a later date, thanks! – user3103598 Dec 16 '13 at 22:33
  • would changing the %i to %c also mean that I would ahve to change the data type for the destination type in the structure? – user3103598 Dec 16 '13 at 22:38
  • No. This change is only for `char` type. No need to change destination type. I edited my answer. There was some typo. Fixed now. This time it will work fine. – haccks Dec 16 '13 at 22:42
  • Just update the main post, have I do what what you said was missing/needed editing. Thanks – user3103598 Dec 16 '13 at 22:49
  • Updated my answer. See it again. Now it is working for me without any crash. – haccks Dec 16 '13 at 22:50
  • changed choice and the %i to %d, still having a problem.... It's soon as I enter a value for the input for &records[*rCount].source, the programme just pumps out an error and then stop. Thanks again for the help. – user3103598 Dec 16 '13 at 22:54
  • what did you also return to stop the crash? Thanks – user3103598 Dec 16 '13 at 22:57
  • I have posted the full code. It is working for option/choice `1` only (as it should). – haccks Dec 16 '13 at 23:01
  • Hi can I ask what OS you are using, as I have just copied and pasted your code into my compiler and it still crashes, wondering if it's the OS. I currently use W7. Thanks for the help still. – user3103598 Dec 16 '13 at 23:08
  • I am using same. WIN7 32-bit. Compiled it with GCC 4.8.1. – haccks Dec 16 '13 at 23:11
  • Could it be something to do with mine been 64bit? As I'm really struggling to get the code working. Thanks – user3103598 Dec 16 '13 at 23:12
  • No. Most probably it should not. Which compiler are you using? – haccks Dec 16 '13 at 23:13
  • I honestly would not know, it's gcc and thats all I know. it came with codeblocks when I downloaded it. Thanks – user3103598 Dec 16 '13 at 23:16
1

What is %i supposed to be doing? Are you looking for an integer? If so, you want %d (d for decimal).

== 1 checks that scanf successfully processed 1 item.

And what @haccks said about missing ).

Phil Perry
  • 2,126
  • 14
  • 18
  • i is supposed to be an integer yes, it is trying to get the integer from user input and store it into the "source" part of the structure. – user3103598 Dec 16 '13 at 22:34
1
struct packet *records;

All well and good but you never actually created a struct packet for this pointer to point to. Therefore all access through this pointer is to invalid memory that does not belong to you.

I don't see any need for a pointer here. Simply declare it as:

struct packet records;

Then pass a pointer to that object:

case 1: system("cls");
    addRecord(&recordCount, &records);

Notice that I've gotten rid of the return for addRecord; you simply do not need it. Make it return void. As it is now, you are taking one invalid pointer and overwriting it with another invalid pointer populated with randomness, since you never actually return anything. It's the same problem, just happening to trigger a crash due to the random value you get.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
  • so putting return records: at the end of the function should clear this. Thanks – user3103598 Dec 16 '13 at 23:10
  • Thank you so much, this has just made it all work! Thank god for that! :) – user3103598 Dec 16 '13 at 23:13
  • still needed *records or the code became invalid for some reason, unless there is a work around for this, again thank you! – user3103598 Dec 16 '13 at 23:15
  • No, in fact you shouldn't return anything. You're already passing the structure in by pointer; no need to return it back in, too. And you _do_ need to declare the actual structure, not just a pointer. "the code became invalid for some reason" is not a clear description of what problem you faced, but at the moment, until you do as I said in my answer, you still have a memory corruption problem which you may not be aware of. – Lightness Races in Orbit Dec 16 '13 at 23:23
  • Ahhh, sorry I am really new to this and trying to explain the problems I face are pretty difficult. I have done what you said and now everything seems to be working well. Thank you – user3103598 Dec 16 '13 at 23:31