Scanf doesn't read string properly in C. What's wrong?

Question

I'm writing a code that's supposed to find the spaces in the string and separate the parts before and after them in different string arrays. The first problem would be that the scanf doesn't even read my string properly, but also I haven't worked with strings in C before and am curious if it's correct (especially with the a[] array).

char expr[50];
char *a[50];
scanf("%s",expr);
int i=0;
int j=0;

while (strlen(expr)!=0){
    if (expr[i]==' '){
        strncpy(a[j],expr,i);
        strcpy(expr,expr+i+1);
        j++;
        i=0;
    }
    else {
        if (strlen(expr)==1){
            strcpy(a[j],expr);
            strcpy(expr,"");
            j++;
            i=0;
        }
        else i++;
    }

}

i=0;

for (i=0; i<j; i++){
    printf("%s\n",a[i]);
}
return 0;

scanf by default stops at whitespaces that are not included in the format string. In your case it will read all characters until the first whitespace character into expr, so if your input is a sentence in english, it will grab the first word. If you're looking to grab a whole line, check out the `fgets` function. — Taelsin, Dec 04 '15 at 00:16
The `%s` conversion specification stops reading at the first white space (blank, tab, newline, etc) — it reads correctly but your expectations are different from its design specification. Either read the line with [`fgets()`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/fgets.html) or POSIX [`getline()`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getline.html) and then use `sscanf()` to parse in stages, or just accept reality and let `scanf()` give you the words one at a time. — Jonathan Leffler, Dec 04 '15 at 00:17
Also `strncpy(a[j],expr,i);` this is bad news, you dont allocate space for the string in `a` this is going to cause `undefined behavior`. — Fantastic Mr Fox, Dec 04 '15 at 00:19
You also haven't allocated space for the `a[j]` pointers to point at, so `strcpy(a[j], expr);` goes horribly wrong. It also isn't clear what makes the loop condition fail; I think you have an infinite loop (well, until you crash for accessing memory out of bounds). — Jonathan Leffler, Dec 04 '15 at 00:20
so I corrected these things, and it reads the string correctly, but it only goes through the while once, and the error message is "Program received signal SIGSEGV, Segmentation fault." — Lilla Nagy, Dec 04 '15 at 00:37
@JonathanLeffler it stops reading at the first whitespace *after* a non-whitespace. Leading whitespace is skipped. — M.M, Dec 04 '15 at 03:25
@M.M: yeah, yeah, yeah…RTFM ([`scanf()`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/scanf.html)) for all the boring details. — Jonathan Leffler, Dec 04 '15 at 03:28

score 1 · Accepted Answer · answered Dec 04 '15 at 00:22

1

This code is wrong.

Firstly, do not use uninitialized a[j].

Add

if((a[j]=calloc(strlen(expr)+1,sizeof(char)))==NULL)exit(1);

before strncpy(a[j],expr,i); and strcpy(a[j],expr); to allocate some memory.

Secondary, strcpy(expr,expr+i+1); is wrong because strcpy() won't accept overlapped regions.

Finally, you should use scanf("%49s",expr); instead if scanf("%s",expr); to avoid buffer overrun.

answered Dec 04 '15 at 00:22

MikeCAT

73,922
11
45
70

so I corrected these things, and it reads the string correctly, but it only goes through the while once, and the error message is "Program received signal SIGSEGV, Segmentation fault." – Lilla Nagy Dec 04 '15 at 00:38
What did you replace `strcpy(expr,expr+i+1);` with? – dxiv Dec 04 '15 at 01:06

score 0 · Answer 2 · answered Dec 04 '15 at 03:35

Don't use scanf, use gets() for standand input or fgets() for reading from a FILE*
To break a string into elements separated by spaces just use strtok():

char expr[50];

gets(expr);

char* a[50];
int i;

for (i = 0; i < 50; i++)
{
    a[i] = (char*)malloc(10); // replace 10 with your maximum expected token length
}

i = 0;

for (char* token = strtok(expr, " "); token != NULL; token = strtok(NULL, " "))
{
    strcpy(a[i++], token); 
}

for (int j = 0; j < i; j++)
{
    printf("%s\n", a[j]);
}


// don't forget to free each a[i] when done.

For simplicity this sample uses deprecated functions such as strcpy, consider replacing it with strcpy_s

`gets` is dangerous and has been deprecated in C99 and removed in C11, long before 2015. [Why is the gets function so dangerous that it should not be used?](https://stackoverflow.com/q/1694036/995714), [Why gets() is deprecated?](https://stackoverflow.com/q/30890696/995714) — phuclv, Feb 06 '21 at 04:43

Scanf doesn't read string properly in C. What's wrong?

2 Answers2