C90 "Aborted (core dumped) massage" (new to C)

Question

First question main program in file set_my.c:

• You must write a program that accepts as input a list of integer values ​​of type int.
• From this list of values you have to build a group.
• The program must receive the list of values ​​from the user, build the group from them, and finally print the the members of the group in the order of their arrival.

Normality of the input can be assumed, but there may be values ​​that repeat more than once in the input list.

For such a value (which appears more than once in the input), its first appearance determines its position in the printed output.

The number of members in the group is not limited and the realloc function must be used.

The number of values ​​in the input is also not limited. The input can be in multiple lines, and each line can appear an unlimited number of entries.

The end of input will be identified by the EOF value returned from a function in the standard library, through which the input is performed (in input from the keyboard, EOF can be inserted by typing d-ctrl at the beginning of a new line).

Input example:

13 13 13 45 -9 -9 -9 18
18 18 3 4 55 45 66 13 66

For this input, the output would be:

13 45 -9 18 3 4 55 66

The output contains only the unique values that appeared in the input, in the order of their first appearance.

The duplicate values are removed.

You are asked to write two functions:

set_get - This function reads input from the user and stores it in a set.

set_print - This function prints the elements of the set in the required order and format.

Here will be my code what I have done so far.

my_set.h

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

#define MAXSIZE  10025

int* get_set();

void replace(int* array, const unsigned int temp, size_t size);

int isDouble(int* array, int find);

void print_set(int* array);

my_set.c

#include "my_set.h"


int main()
{
    int* set;
    set = get_set();
    print_set(set);
    return 0;
}
int* get_set2(char* x);
/*the requested function, as specified in the assignment.*/
int* get_set()
{

    int sizeOfStr;
    char* str;
    char* output;
    str = malloc(sizeof(char) * MAXSIZE + 1);
    output = calloc(1, sizeof(char)); /*which will store the valuby information of str, before it restarts to defult settings*/
    /*We assumed that our maximum capacity is MAXSIZE which is equal to 10025  */
    /*We will literate through the provided list of numbers untill a null value is encountered*/
    while (fgets(str, MAXSIZE, stdin) != NULL)
    {
        sizeOfStr = strlen(str);
        output = realloc(output, sizeOfStr * sizeof(char) + sizeof(output)+1);
        output = strcat(output, str);

    }
    return get_set2(output);
} 

int* get_set2(char* x)
{
    int counter;
    int* array;
    counter = 0;
    array = calloc(1, sizeof(int));
    /*Prior to proceeding further, it is imperative to verify if the input received is valid, as continuing without valid input would not be desirable.*/
    while (counter < strlen(x))
    {
        if (!((*x >= (int)'0' && *x <= (int)'9') || *x == ' ' || *x == '-'))
        {
            printf("There was an error while checking for valid acception of letter, therefore not valid letter");
            return NULL;
           
        }
        /*In this step, we will modify it to convert valid character numbers into integers.*/
        /*The function initially removes any leading whitespace characters in a systematic manner, prior to proceeding with the atoi operation.*/
        /*If the value of counter is not equal to zero, we will need
        to request additional space using realloc otherwise we don't need additional space..*/
        if (counter != 0)
        {
            array = realloc(array, sizeof(int) * counter + sizeof(array));
           *array = atoi(*x);
                
        }
        else
        {
            *array = atoi(*x);
        }
        counter++;
    }
    /*From this point onwards,our foucs will be on eliminating duplications in our list.
       With the assistance of two functions, "replace" and "isDouble", we will work towards
       removing duplications.*/
    counter = 0; /*Restarting to defult settings of parameter counter*/
    int temp;
    int arraySize;
    arraySize = sizeof(*array) / sizeof(array[0]);
    temp = 0;
    while (counter < arraySize)
    {
        if ((temp = isDouble(array, array[counter])) != -1)
            replace(array, temp, arraySize);

            counter++;
    }

    return x;
}


void replace(int* array, const unsigned int temp, size_t size)
{
    int* current = array + temp;
    int* end = array + size;

    /* If the size of the array is determined to be 0, there will be no further action required.*/
        if (size == 0)
            return;

    /* If the index that needs to be removed corresponds to the last element in the array
    a efficiently reallocate the array with one less element and subsequently return the updated array. */
        if (temp == size)
        {
            array = realloc(array, (size - 1) * sizeof(int));
            return;
        }

    /* Alternatively, if the size of the array is greater than 0, it would necessitate
    shifting all the elements following the removed element by one position to the left. */
        while (current < end)
        {
            *current = *(current + 1); /* Setting the current element to the next element can be accomplished via assignment.*/
            current++; /* Moving to the next element simply requires transitioning to the subsequent item in the sequence. */
        }
    /*At last, we have the opportunity to reallocate the array by reducing its size by one element. */
        array = realloc(array, (size- 1) * sizeof(int));
}
int isDouble(int* array, int find)
{
    size_t size = sizeof(array) / sizeof(array[0]);
    int i;
    unsigned int counter = 0; /* Counts the occurrences of find*/

        /*If the size is zero, return -1 without performing a search. */
        if (size == 0)
        {
            return -1;
        }

    /* Iterate through the array and tally the occurrences of the given number.
    If it appears more than once, return the index of the first occurrence. */
        for (i = 0; i < size; i++)
        {
            if (counter > 1 && *array == find)
                return i;
            else
            if (*array == find)
                counter++;
        }

    /* If the number is not found or appears only once, return -1 */
        return -1;
}

void print_set(int* array)
{
    int size;
    size = sizeof(array) / sizeof(array[0]);
    for (int i = 0; i < size; i++) {
        printf("%d' '", array[i]);
    }
}

And here is the bug I am getting exactly after 8 characters. Would love some help, I got not idea how to advance.

Answer 1

The biggest issue you are facing is your code is horribly overly complicated for the problem you are trying to solve. Try breaking what you actually need to do down into small simple steps and then implement concise code that does just that. Your direction specify two functions set_get() and set_print() as you describe in your question. You have the freedom to choose the type for each function as well as the parameters each take.

For set_group(), you simply need to:

• loop reading int from stdin,
• check if the value read is already in your group (your set),
• if it is -- skip to read the next value,
• otherwise, realloc your group to hold another int and assign the value read to the newly allocated storage for int.

For set_print(), you simply need to:

• loop over each value in your group,
• outputting the value to stdout (with a space before the 2nd output on),
• tidy up with a newline.

To accomplish both, you need only keep track of the start address for your group and the number of members (int) in the group. You are always free to use additional temporary value, but try and keep the variable sprawl to a minimum.

How do you read an int from stdin? (where there can be an unknown number of whitespace separated integers in the input buffer). There is a function for that -- scanf(). Generally, you would want to read input with fgets() into a buffer and then parse the buffer, either repeatedly calling strtol(), or looping with sscanf() using the "%n" specifier to keep track of offset, or several other methods. But here you are told "Normality of the input can be assumed...", so KISS (keep it stupid simple) and just use scanf() (properly validating the return).

So what might set_get() look like?

int *set_get (int *nmemb)
{
  int n = 0,              /* number of int in group */
      val,                /* temporary value to hold input */
      *group = NULL;      /* pointer to mem holding group */
  
  /* while int read from stdin */
  while (scanf ("%d", &val) == 1) {
    int in_group = 0;                 /* flag indicating val exists in group */ 
    
    for (int i = 0; i < n; i++) {     /* loop over int in group */
      if (val == group[i]) {          /* if value already in group */
        in_group = 1;                 /* set flag true */
        break;                        /* break loop */
      }
    }
    
    if (!in_group) {                  /* if val not in group */
      /* always realloc using temp pointer */
      void *tmp = realloc (group, (n + 1) * sizeof *group);
      if (!tmp) {                     /* validate every allocation */
        perror ("realloc-group");     /* handle error */
        break;                        /* break read-loop */
      }
      group = tmp;                    /* assign realloc'ed block to group */
      group[n++] = val;               /* assign val to group, increment n */
    }
  }
  
  *nmemb = n;           /* update value of nmemb in memory */
  
  return group;
}

note: while up to you, the '*' declaring a pointer generally goes with the pointer not the type. Why? Because:

int* a, b, c;

... does not declare three pointers-to int (it declares one pointer a and two int, b, c. Writing:

int *a, b, c;

... makes that clear.

In the set_get() function, note how a pointer-to int is returned allowing you to return a pointer to your allocated group (or NULL on failure of realloc() before the first valid int is stored. Also note how the reallocation is ALWAYS done to a temporary pointer. That way when (not if) realloc() fails and returns NULL, you don't overwrite your pointer address creating a memory leak.

(note: While the simple function above reallocates for each integer -- you would generally want to grow your block of memory by more than one int keeping track of the storage for the number of integer allocated and the number used and only reallocate when used == allocated. Doubling the size of the allocation each time a realloc() is needed is one such method that balances the number of calls to realloc() and memory growth in a reasonable way)

You could also have chosen to pass a pointer-to-pointer to realloc() within the function returning the number of int in your group (and, e.g. -1 on failure). Since the operations within the function can either succeed or fail you must return a type capable of indicating which happened. Choosing void for the type of set_get() would not work unless you passed pointers for parameters for all values and checked those after return in lieu of the checking return itself -- that is a less straight-forward approach generally used in call-back functions.

One other area where you have a misconception is in your void print_set(int* array). You cannot use size = sizeof(array) / sizeof(array[0]); to get the number of elements in array. Why? array is passed as a pointer, it's no longer an array (see C18 Standard - 6.3.2.1 Other Operands - Lvalues, arrays, and function designators(p3) - on access array is converted to pointer to it's first element). So what you are really doing is size = sizeof (a_pointer) / sizeof (int); Which for x86_64 is 8 / 4 = 2 or for x86 4 / 4 = 1. You can only use sizeof array / sizeof array[0] in the scope where array is declared. (otherwise, it's just a pointer)

Now what of your set_print() function?

Here you are just outputting the stored information. What happens in set_print() isn't critical to the continued operation of your program. The choice of void for the return-type for set_print() is fine. All this function needs is a pointer to your group (e.g. the address of the first int in your group) and then number of int to print, e.g.

void set_print (int *group, int nmemb)
{
  for (int i = 0; i < nmemb; i++) {         /* loop over each int in group */
    printf (i ? " %d" : "%d", group[i]);    /* output w/' ' before 2nd on */
  }
  
  putchar ('\n');       /* tidy up with \n */
}

Your main() function is then quite simple, e.g.

int main (void) {
  
  int n = 0,                    /* number of int in group */
      *group = set_get (&n);    /* allocated block holding group */
  
  set_print (group, n);         /* print the int in group */
  
  free (group);                 /* don't forget to free allocated mem */
}

Add:

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

to the top of the two functions and main() and you have a complete program.

Example Use/Output

$ ./set << EOF
> 13 13 13 45 -9 -9 -9 18
> 18 18 3 4 55 45 66 13 66
> EOF
13 45 -9 18 3 4 55 66

(if you are on Window and do not have a heredoc available to redirect input on stdin, just echo the multiple lines and pipe that output to your program -- or store the values in a file and redirect the file as input)

Memory Use/Error Check

In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.

It is imperative that you use a memory error checking program to ensure you do not attempt to access memory or write beyond/outside the bounds of your allocated block, attempt to read or base a conditional jump on an uninitialized value, and finally, to confirm that you free all the memory you have allocated.

For Linux valgrind is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it.

$ ./set << EOF
13 13 13 45 -9 -9 -9 18
18 18 3 4 55 45 66 13 66
EOF

==11741== Memcheck, a memory error detector
==11741== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==11741== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==11741== Command: ./bin/set
==11741==
13 45 -9 18 3 4 55 66
==11741==
==11741== HEAP SUMMARY:
==11741==     in use at exit: 0 bytes in 0 blocks
==11741==   total heap usage: 10 allocs, 10 frees, 5,264 bytes allocated
==11741==
==11741== All heap blocks were freed -- no leaks are possible
==11741==
==11741== For lists of detected and suppressed errors, rerun with: -s
==11741== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Always confirm that you have freed all memory you have allocated and that there are no memory errors.

Look things over and let me know if you have additional questions.

---

Improve / Refactor / Optimize - After Your Code Is Complete

Since you are learning, understand writing code is an iterative process. Nobody writes perfect code for the first version. After you have you code working, go back and think about where you can make improvement -- for efficiency or for readability.

In this case, I would probably break in_group out of the set_get() function and make in_group() a function that checks whether a given value exists in the allocated block, making the return type int returning 1 if the value exists in the group, 0 otherwise.

For me, that just makes both functions more readable and easier to maintain. I can also use in_group() in other code making it reusable.

The bottom line rule is "Get the code working correctly" then "worry about optimizations and tidying up readability". Putting that rule to work, you could add an in_group() function and rewrite set_get() as

/* check if val exists in allocated group of nmemb members,
 * returns 1 if val exists in group, 0 otherwise.
 */
int in_group (int val, int *group, int nmemb)
{
  for (int i = 0; i < nmemb; i++) {   /* loop over int in group */
    if (val == group[i]) {            /* if value already in group */
      return 1;                       /* return true */
    }
  }
  
  return 0;       /* return false */
}

Look at the simplification that allows to set_get():

/* repeatedly read unique int from stdin into dynamically allocated block of
 * memory. Returns pointer to allocated block of memory on success with
 * number of members (nmemb) updated to provide the number of int in group.
 * Returns NULL if no integer was available or if allocation failed before
 * first int was stored.
 */ 
int *set_get (int *nmemb)
{
  int n = 0,              /* number of int in group */
      val,                /* temporary value to hold input */
      *group = NULL;      /* pointer to mem holding group */

  /* while int read from stdin */
  while (scanf ("%d", &val) == 1) {
    if (in_group (val, group, n)) {   /* if val in group, get next val */
      continue;
    }
    
    /* always realloc using temp pointer */
    void *tmp = realloc (group, (n + 1) * sizeof *group);
    if (!tmp) {                       /* validate every allocation */
      perror ("realloc-group");       /* handle error */
      break;                          /* break read-loop */
    }
    group = tmp;                      /* assign realloc'ed block to group */
    group[n++] = val;                 /* assign val to group, increment n */
  }

  *nmemb = n;           /* update value of nmemb in memory */

  return group;
}

How you optimize and refactor/rewrite is completely up to you. Let me know if you have additional questions.