0

I'm trying to write a program to take 10 words as input from the user, then store it an array, and then print out the length of each word.

Here's my code:

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

char *words[10];
char length[10];
int i, j;

int main()
{
    printf("Input ten words: \n");

    for(i = 0; i < 10; i++)
    {
        printf("Enter element %d \n", i + 1);
        scanf("%s", &words[i]);
    }

    for(i = 0; i < 10; i++)
     printf("%c", words[i]);

    for(i = j = 0; j < 10; j++)
    {
        length[j] = strlen(words[i]);
        i++;
    }

    for(j = 0; j < 10; j++)
        printf("%c", length[j]);

    return 0;
}

It should be noted that I have no idea why the array "words" is defined as a pointer, I only do it because if I don't I get some warning about making a pointer from integer without a cast.

When I run the program what happens is, I get prompted to input the 10 elements, that much works, but then when it's supposed to print the "words" array, the program just crashes.

Also the reason I coded it like this is because later on I also need to print the longest and shortest word - so I figured it would help if I had the lengths of all the strings in their own array.

Does anyone know what's wrong here?

Thanks

Wacy
  • 43
  • 7
  • 2
    `char *words[10];` is an array of pointers you have to allocate it first before using. – IrAM Dec 10 '20 at 19:51
  • 1
    Just as a side note: It is unsafe to use `scanf` without checking the return value. See this page for further information: [A beginners' guide away from scanf()](http://sekrit.de/webdocs/c/beginners-guide-away-from-scanf.html) – Andreas Wenzel Dec 10 '20 at 19:52
  • When you get the declaration and usage or `words` fixed, you will discover that you do not want to use `scanf` like this. Consider the behavior when a line of input contains whitespace. – William Pursell Dec 10 '20 at 20:51

1 Answers1

0

With the line char *words[10], you are declaring an array of 10 pointers. However, these pointers are uninitialized, which means they are wild pointers. Dereferencing a wild pointer causes undefined behavior (i.e. the program may crash). If you want to use these pointers in a meaningful way, you must make each pointer point to a valid memory location, for example to an address returned by the function malloc or to the address of a char array.

However, probably the easiest solution to your problem is to not use pointers at all, but to instead declare a two-dimensional char array, like this:

char words[10][100];

That way, you are allocating space for 10 words of up to 100 characters each (including the null terminating character).

Beware that a buffer overflow will occur if the user enters a word longer than 99 (1 byte is required for the terminating null character). Therefore, the scanf line should be changed to the following:

scanf("%99s", words[i]);

That way, scanf will never attempt to write more than 100 bytes (including the terminating null character).

I have also removed the & in the scanf line above, because the & is not necessary, since words[i] will automatically decay to &words[i][0].

Also, as a general rule, you should verify that the return value of scanf is 1 before attempting to use the value that scanf wrote. For example, if the user triggers end of file on the input stream (for example by pressing CTRL-D on Linux or CTRL-Z on Windows), then scanf will return -1 without writing anything into words[i]. In that case, by subsequently reading from words[i], your program will cause undefined behavior.

Additionally, the line

printf("%c", words[i]);

must be changed to:

printf("%s", words[i]);

The loop

for(i = j = 0; j < 10; j++)
{
    length[j] = strlen(words[i]);
    i++;
}

can be simplified to:

for(i = 0; i < 10; i++)
{
    length[i] = strlen(words[i]);
}

The line

printf("%c", length[j]);

should probably be changed to

printf("%hhu", length[j]);

because length[j] does not represent the ASCII code of a character, but just a number.

Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39