0

I wanted to expand an array dynamically as the user enters the values using below code.

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

int main(){
    int *number=(int *)malloc(sizeof(int)),i=0,*new_ptr;
    printf("Enter a number to store it or -1 to finish the storage\n");
    scanf("%d",number);
    while(*(number+i)!=-1){
        printf("Enter a number to store it or -1 to finish the storage:\n");
        number=(int *)realloc(number,sizeof(number)+sizeof(int));
        i++;
        scanf("%d",number+i);
    };
    for(int j=0;j<i;j++){
        printf("%d ",number[j]);
    };
    return 0;
}

it gives this output

Enter a number to store it or -1 to finish the storage
1
Enter a number to store it or -1 to finish the storage:
2
Enter a number to store it or -1 to finish the storage:
3
Enter a number to store it or -1 to finish the storage:
4
Enter a number to store it or -1 to finish the storage:
5
Enter a number to store it or -1 to finish the storage:
6
Enter a number to store it or -1 to finish the storage:
7
Enter a number to store it or -1 to finish the storage:
8
Enter a number to store it or -1 to finish the storage:
9
Enter a number to store it or -1 to finish the storage:
0
Enter a number to store it or -1 to finish the storage:
-1
1 2 3 4 5 6 7 8 540155953 540287027

the junk file values happens only when the number of user input is greater than 8. Why it happens and how to fix it?

10
  • 6
    sizeof(number) is not what you think it is. sizeof(number) is the size of the pointer pointing to the allocated memory. Commented Dec 10, 2023 at 16:12
  • If you have i numbers, how many int elements do you need to store them? If one int element takes sizeof (int) bytes, how many bytes do you need to store i elements of type int? What argument should you pass to realloc to request that many bytes? Commented Dec 10, 2023 at 16:13
  • 1
    Do i++; before number = realloc(number, sizeof *number * (i + 1)); Commented Dec 10, 2023 at 16:45
  • 3
    Assign the new pointer to a temporary variable, take action if it is NULL else assign that to number. Commented Dec 10, 2023 at 16:57
  • 1
    A call to scanf which discards the return value is always a bug. You must always check the value returned, whether it is simply if( scanf(...) or while( scanf(...) you must not discard the return value. Commented Dec 10, 2023 at 17:08

2 Answers 2

1

There were a few small issues with the original code. Junk values got printed because the original code invoked undefined behavior. Specifically, realloc() was not allocating enough space in the old logic. realloc() needs to know new total space.

Original code was always realloc()ing sizeof(number)+sizeof(int)... which translates to sizeof(int *) + sizeof(int).

That combo results in a constant which does not grow. This may at least partly explain why you could get OK performance until the array actually needed more space than sizeof(int *) + sizeof(int). The new code fixes that by updating the allocation size logic.

Other adds/updates:

  • we check return of scanf() (best practice)
  • we check return of realloc() before applying it to our number pointer so we don't lose the number pointer reference.
  • we do not cast realloc() return to (int *), it is not required
  • Other improvements documented in comments.
  • This code is a conglomerate and manifestation of several great points made by the community, as well as some uniquely my own. Please enjoy:
    #include <stdio.h>
    #include <stdlib.h>

    int main()
    {
        int *number=malloc(sizeof(int)), *temp, i=0,j, sres,scap;
      
        if(!number)
        {
            /* malloc() problem.  Alert user and exit. */
            printf("Coule not allocate memory!\n");
            return 0;
        }

        /* The do / while() loop design eliminates redundancy of the 
        ** scanf() prompt and fetch.  We prompt user and scan() 
        ** in a single code block. */
        do {
            printf("Enter a number to store it or -1 to finish the storage:\n");
            /* Use a separate, new variable to capture the scanf() input */
            sres = scanf("%d", &scap); /* Added: always check the return of scanf()*/
            if(sres!=1)
            {
                /* scanf() problem.  Alert user and exit. */
                printf("scanf() error!\n");
                return 0;
            }
            /* New logic/sequence.  If the captured value is not -1, store and continue*/
            if(scap != -1)
            {
                number[i++] = scap;  /* Store input to array */
                /* Updated allocation size logic: (i+1) means current count + next input: */
                temp = realloc(number, (i+1)*sizeof(number)); 
                if(temp)
                {
                    /* Preserve number pointer.  Only assign new 
                    ** memory if allocation was successful*/
                    number = temp; 
                }
                else
                {
                    /* realloc() problem.  Alert user and exit. */
                    printf("Coule not allocate memory for next input!\n");
                    return 0;
                }
            }
        }while(scap != -1);  /* Condition modified to adapt to new logic*/

        for(j=0;j<i;j++)
        {
            printf("%d ",number[j]);
        }

        return 0;
    }


Output:

Enter a number to store it or -1 to finish the storage:
1
Enter a number to store it or -1 to finish the storage:
2
Enter a number to store it or -1 to finish the storage:
3
Enter a number to store it or -1 to finish the storage:
4
Enter a number to store it or -1 to finish the storage:
5
Enter a number to store it or -1 to finish the storage:
6
Enter a number to store it or -1 to finish the storage:
7
Enter a number to store it or -1 to finish the storage:
6
Enter a number to store it or -1 to finish the storage:
5
Enter a number to store it or -1 to finish the storage:
4
Enter a number to store it or -1 to finish the storage:
3
Enter a number to store it or -1 to finish the storage:
2
Enter a number to store it or -1 to finish the storage:
1
Enter a number to store it or -1 to finish the storage:
-1
1 2 3 4 5 6 7 6 5 4 3 2 1 
Sign up to request clarification or add additional context in comments.

Comments

1

For starters the variable new_ptr is not used. Always try to declare variables in minimum scopes where they are used.

In this call of realloc

number=(int *)realloc(number,sizeof(number)+sizeof(int));

the expression sizeof(number) is equivalent to expression sizeof(int *) due to the type of the variable number

int *number=(int *)malloc(sizeof(int)),i=0,*new_ptr;
^^^^^

Thus the exoression sizeof(number)+sizeof(int) is a constant that is not being changed within the while loop. It can be equal for example to 12 if sizeof( int * ) is equal to 8 and sizeof( int ) is equal to 4. In this case there is allocated memory only for 3 integers. Adding more integers in the loop results in undefined behavior due to accessing memory outside the allocated extent of memory. You need to increase the allocated memory for new added integers. Also it is a bad idea to allocate memory initially before the loop because the user can at once enter -1. And in any case you should free the allocated memory when it is not required any more.

And you should check success of failure of calls of scanf and realloc.

The program can look the following way. Pay attention to that the function malloc even is not used in the program because it is redundant. Also if memory can not be reallocated there is no reason to exit the program. You can output those numbers that were already successfully stored.

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

int main( void )
{
    int *number = NULL;
    size_t n = 0;

    int success = 0;
    do
    {
        int value;

        puts( "Enter a number to store it or -1 to finish the storage" );

        success = scanf( "%d", &value ) == 1 && value != -1;

        if (success)
        {
            int *new_ptr = realloc( number, ( n + 1 ) * ( sizeof( *new_ptr ) ) );
            success = new_ptr != NULL;

            if (success)
            {
                number = new_ptr;
                number[n++] = value;
            }
            else
            {
                puts( "The input is interrupted. No enough memory to store numbers" );
            }
        }
    } while (success);

    for (size_t i = 0; i < n; i++)
    {
        printf( "%d ", number[i] );
    }
    putchar( '\n' );

    free( number );

    return 0;
}

The program output might look like

Enter a number to store it or -1 to finish the storage
1
Enter a number to store it or -1 to finish the storage
2
Enter a number to store it or -1 to finish the storage
3
Enter a number to store it or -1 to finish the storage
4
Enter a number to store it or -1 to finish the storage
5
Enter a number to store it or -1 to finish the storage
6
Enter a number to store it or -1 to finish the storage
7
Enter a number to store it or -1 to finish the storage
8
Enter a number to store it or -1 to finish the storage
9
Enter a number to store it or -1 to finish the storage
0
Enter a number to store it or -1 to finish the storage
-1
1 2 3 4 5 6 7 8 9 0

Comments

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.