0

So I have this program where you first create an array and then you have to input integers in the array by using pointers. the integers have to be different. Now, my problem is when the program starts to check for the elements to check for any existing integers it crashes...I don't know what to I am doing wrong.

Source file:

/*source1.c - using an ADT array*/
/*compile with main.c*/
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#include "header1.h"/*defines array*/
int Create()
{
   int *ptr;
   ptr=(int*)malloc(sizeof(int));
   *ptr==0;
   return ptr;
}
bool is_element_of(int x,int *array1)
{
    int i;
    i=0;
    int n=0;
    int flags=0;
    while (array1[i]!=NULL)
    {
        if (x==*array1)
        {
            printf("%d is element of set",x);
            flags=1;
            n++;
        }
        else
        { 
            array1++;
            n++;
        }
    }
}
void Add(int *array1,int x)//add an element to set S
{
    int*ptr;
    ptr=array1;
    int n;
    if(array1==NULL)
    {
        puts("Memory Allocation Failed! Goodbye!");
        exit(EXIT_FAILURE);
    }
    int i;
    int flags;
    printf("Enter an integer: ");
    scanf("%d",x);
    bool is_element_of(int x,int *array1);
    printf("x");
    while(flags==1)
    {
        printf("Error! Please enter a different integer!");
        scanf("%d",x);
        bool is_element_of(int x,int *array1);
    }
    ptr=realloc(ptr,n*sizeof(int));
    array1=x;
    free(array1);
}

Header file:

//Header1.h - Array Implementation header file
#ifndef HEADER1_H_INCLUDED
#define HEADER1_H_INCLUDED
#include <stdbool.h>
//initializing the array
struct memory
{
    int array1;
};
typedef struct memory Item;

//creates a new empty set
int Create();

//the function that will adds x to array S, if not present
void Add(int *array1,int x);
#endif
6
  • You aren't asking for c++, do you? Re-Tagged ... Dec 20, 2013 at 15:22
  • 1
    In your create function *ptr==0; probably '='? Also, it should return an int, whereas it returns a pointer. Dec 20, 2013 at 15:22
  • if i do *ptr=0; it just gives me the error of the Memory Allocation..
    – user3123178
    Dec 20, 2013 at 15:27
  • That's because there are tons of other bugs in your code. Dec 20, 2013 at 15:32
  • 1
    You don't use Create(); it is a red herring. Remove it. You redeclare is_element_of() after the call to scanf(), which does nothing useful. In fact, you do that twice! Maybe you meant to call the function instead? You don't test that your scanf() calls succeed. You don't tell the is_element_of() function how big the array currently is; it has to intuit it, and computers are bad at intuition. AFICS, you don't use the struct memory or Item types. Dec 20, 2013 at 15:47

3 Answers 3

2

I may get told "this is not an answer", but I think it's important:-

To solve the problems in your code (yes, there are many), you need to debug it. and you need to learn how to do that. You can do things like 'dry runs' by working through the code flow with pen and paper, or you can use a debugger.

A debugger will allow you to step through the code, set breakpoints and inspect variabled at any point. It's one of the most useful skills you can learn, and is almost essential for any non-trival programming problem. StackOverflow is NOT your debugger ;-)

Start by finding out about the debugger on your system, step through your code, and if you find a specific problem with some code that you can't understand, ask about it here - with details of what your debugger is showing you, and what you expected it to show.

And as @floris says, there's more you can do before you use a debugger : Read all the compiler's warnings, and treat the compiler as a friend, not an enemy to be beaten into submission - Turn all warnings on, and don't ignore them. Good code compiles with ZERO warnings. You may also have a tool called lint on your system, which usually does even deeper checking.

1
  • 1
    Even before running the debugger, turn on all compiler warnings and HEED THEM. Declaring a bool function and not having a return statement should cause a message, at least. As should comparing an int with a void *, etc...
    – Floris
    Dec 20, 2013 at 15:48
1

One of the "many bugs" is in this function:

bool is_element_of(int x,int *array1)
{
    int i;
    i=0;
    int n=0;
    int flags=0;
    while (array1[i]!=NULL)
    {
        if (x==*array1)
        {
            printf("%d is element of set",x);
            flags=1;
            n++;
        }
        else
        { 
            array1++;
            n++;
        }
    }
}

This loop indexes array1[i] but increments n. So you have an infinite loop. And there's more: You test the contents of the array (an int)for equality with a pointer (NULL) - a different data type. It sets flags, but it returns nothing (although you declared a return type).

It would be helpful to pass the size of the array to your functions so they don't exceed the boundaries. For this function, you might address all the above by re-writing as:

bool is_element_of(int x, int *array1, int n)
{
    int ii;
    for(ii = 0; ii < n; ii++) {
      if( x == array1[i]) return true;
    }
    return false;
}

If you have to do it with pointers:

bool is_element_of(int x, int *array1, int n)
{
    int ii;
    for(ii = 0; ii < n; ii++) {
      if( x == *(array1 + i)) return true;
    }
    return false;
}

And if you want to be really tricky:

bool is_element_of(int x, int *array1, int n)
{
    int ii;
    for(ii = 0; ii < n; ii++, array1++) {
      if( x == *array1) return true;
    }
    return false;
}

There are plenty of other problems, but maybe this gives you some inspiration: check that you don't run past the end of your array, make sure you are updating your index correctly, ...

EDIT another significant problem of understanding happens in your Add function - and it probably causes the crash before you even check that the element is in the array:

printf("Enter an integer: ");
scanf("%d", &m);  // you need to pass the address of the variable, not the value

Also - keep track of not only the size of the array, but also the number of elements you have stored so far: this way you can add a new value to the end, and when you have run out of space the program can (should) stop.

It's a good practice program; you have a lot to learn...

1
  • thanks a lot for your help guys! i really appreciate it! i know it has a lot of bugs but i will keep at it until it is done.
    – user3123178
    Dec 20, 2013 at 15:59
0

When you call scanf, you need to pass a reference to x, not x itself. That line should look like scanf("%d",&x). There is also no reason for x to be a parameter of the function since it is overwritten by the scanf call before it is used; make it a local variable instead.

The variable flags is never initialized, meaning the condition in the while loop will never be true (unless x happens to be 0, but you don't know, because it's not initialised). I assume that you intended flags to be set by the call to is_element_of, in which case two things need to happen: is_element_of needs to have return flags at the end-currently, it does not return a value despite being declared as such.

Second, the call to is_element_of needs to be flags=is_element_of(x,array1);. The existing line bool is_element_of(int x,int *array1); declares the function again but does not call it.

1
  • Can you explain to me what did you mean when you said "is_element_of needs to have return flags at the end-currently, it does not return a value despite being declared as such."? Thanks
    – user3123178
    Dec 20, 2013 at 15:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.