Output is printing twice, why?-CodePudding

The program is meant to remove the '-' from an ISBN code inputted, eg. "978-123456-789" is inputted and "978123456789" is outputted. Instead what I'm getting out is "978123456789978123456789" - it's printing it twice. Can someone please explain to me why? Thanks

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

int main(void)
{
    char ISBN[16], arrayClean[12];
    int i,j,k,a;

    printf("Enter your ISBN: ");
    scanf("%s",&ISBN);

    for(i=0; i<=13; i  )
    {
        a = ISBN[i] - 48;
        if(a==-3)
        {
            for(j=i;j<=13;j  )
            {
                k  ;
                ISBN[j]=ISBN[j 1];
            }
            k=0;
            i=0;
        }
    }

    for(i=0; i<=11; i  )
        arrayClean[i]=ISBN[i];

    printf("%s",arrayClean);


    return 0;
}

CodePudding user response：

You seem to have 12 chars in a number (excluding dashed. 14 counting them).

Your loops therefore cannot deal with chars 0 to 11 of the output, and chars 0 to 13 of the input. That is forgetting the terminal '\0' that needs to be there also, in the output, at position 12.

(If you are 100% sure there will be 12 chars, then you could solve this simply by adding arrayClean[12]=0 at the end, just before printing. But that would be a bad idea. Since it would be relying on what is typed by the user).

Also, even the declarations of your array "arrayClean" does not take into account the need for a terminal '\0'. You need 13 bytes to hold a 12 characters string.

Some other remarks:

Your usage of scanf("%s", &ISBN); is dangerous.

First of all, it should be scanf("%s", ISBN);. What you need to pass scanf is the address where to store what it reads. When you are reading a single int x;, then, indeed, you should scanf("%d", &x);, to have scanf store the result at the address where x is stored. But for a string, ISBN is already an address (the address of chars ISBN[0], ISBN[1], ...). So you should not pass the address of ISBN.

What saves you here, is that since ISBN is not a variable (it is an array, that is a constant pointer), in C &ISBN and ISBN are the same thing in this very context. So, it happens to work by accident.

But if ISBN were a variable (a char * allocated with malloc for example), then this usage would lead to memory errors. So, you should take the habit of not passing &ISBN to scanf but ISBN.

Other problem with that same line: when scaning, you cannot trust the user to type the exact number of bytes you expect them to. Here, if the user types 16 or more bytes, those byte will be written to whatever memory is after the 16 bytes of ISBN. Which will cause very serious problem. Even security ones if the user does this on purpose to overwrite other variables and even codes on some architecture.

So, rule of thumb: never ever scanf("%s", ...). Always enforce a limit to the number of bytes scanf can read. scanf("s", ...) for example.

CodePudding user response：

You are not null terminating the string. You are only copying 12 characters from ISBN to arrayClean. You need to add a null terminator to the end of arrayClean. You can do this by adding arrayClean[12] = '\0'; after the for loop.

char ISBN[16], arrayClean[12];
int i, j, k, a;

printf("Enter your ISBN: ");
scanf("%s", &ISBN);

for (i = 0; i <= 13; i  )
{
    a = ISBN[i] - 48;
    if (a == -3)
    {
        for (j = i; j <= 13; j  )
        {
            k  ;
            ISBN[j] = ISBN[j   1];
        }
        k = 0;
        i = 0;
    }
}

for (i = 0; i <= 11; i  )
    arrayClean[i] = ISBN[i];

arrayClean[12] = '\0';

printf("%s", arrayClean);