0

In what way is the following code faulty or undefined behaviour?

I suggested this as a possibility to create an array of strings, if string number and size are unknown beforehand, and after a short discussion, it was suggested that I open a new question.

The following code produced the expected results when I compiled it with gcc, but that can happen in spite of undefined behaviour (it's undefined after all).

So, what is the mistake?

int n, i;

printf("How many strings? ");
scanf("%d", &n);

char *words[n];

for (i = 0; i < n; ++i) {
    printf("Input %d. string: ", i + 1);
    scanf("%s", &words[i]);
}

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

Edit:

I feel stupid now for missing this, I guess getting the correct answer back just made me miss my error. But that others may learn from my mistake: I guess I got completely wrong what the & operator does. I thought it would give me where words points to, but of course it does the exact opposite. See the answers.

  • `printf("%s\n", words[i]);` – Cid Aug 27 '20 at 13:55
  • 1
    Please explain where you think that the scanf writes the scanned strings. That info is needed to know which detail level of "your code does not make sense" you still understand and which is the interesting part. I.e. do you think the strings are scanned into the memory represented by `words` (with totally wrong types being ignored) or do you think the strings are written to locations pointed to by the members of `word` (and just fail to make them point to legal memory). – Yunnosch Aug 27 '20 at 13:56
  • 1
    You have an array of pointers, but where do each pointer actually point? – Some programmer dude Aug 27 '20 at 13:57
  • 1
    Also, since `words[i]` is the type `char *`, when you're doing `&words[i]` you get the type `char **`, which is totally wrong for the `%s` format(which expects a pointer to the first element of an array of characters, i.e. a `char *`). – Some programmer dude Aug 27 '20 at 13:58
  • I'm glad that you finally understood it and that my suggestion was successful even if you didn't understood what I said in the first place. – RobertS supports Monica Cellio Aug 27 '20 at 15:22

3 Answers3

1

scanf("%s", &words[i]); and printf("%s\n", &words[i]); invokes *undefined behavior because data having wrong type are passed.
In both of scanf() and printf(), %s requires char* pointing at valid buffer but what is passed are char**.

Also don't forget to allocate buffer to store strings before reading.

Try this:

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

int main(void) {
    int n, i;

    printf("How many strings? ");
    scanf("%d", &n);

    char *words[n];

    for (i = 0; i < n; ++i) {
        printf("Input %d. string: ", i + 1);
        words[i] = malloc(1024001); /* allocate enough buffer */
        if (words[i] == NULL) {
            perror("malloc");
            return 1;
        }
        scanf("%1024000s", words[i]); /* limit length to read to avoid buffer overrun */
    }

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

    for (i = 0; i < n; ++i) {
        free(words[i]); /* clean what is allocated */
    }
    return 0;
}
MikeCAT
  • 73,922
  • 11
  • 45
  • 70
1
char *words[n];

creates an array of uninitialized pointers

scanf("%s", foo);

writes values to the position foo is pointing to

it is not specified where the pointers of words are pointing to so they could point anywhere which could result in a segfault

next words is a char**

words[i] is a char *

&words[i] is a char **

%s expects a char* so it's again undefined behavior what happens

so you first have to initialize your words arrays using for example malloc and then write the values to words[i]

HED
  • 83
  • 1
  • 5
  • `&words[i]` is `char**` but `words` is not. It is an array of pointers and not a pointer to pointer. – klutt Aug 27 '20 at 14:24
  • words is a char ** if you write `int * x` you push a pointer to the stack that points to nothing if you write `int x[2]` you push two integer values to the stack and x points to the first of it, so x is equivalent to `&x[0]` the compiler even tells you that. If you use `char *words[5];scanf("%s",words);` it will throw the warning `%s expects char * but got char **` – HED Aug 27 '20 at 14:41
  • No, they are not equivalent. However, when passed as an argument to a function like that, an array will decay to a pointer. But try `printf("%zu %zu\n", sizeof(char *[n]), sizeof (char**));` for instance, and you will see that they are different. – klutt Aug 27 '20 at 14:45
  • Also, you could try doing something like `char *words[n]; char **ptr = NULL; words = ptr;` which would not work, because `words` is an array and not a pointer. – klutt Aug 27 '20 at 14:49
1

This:

char *word;

is a pointer, Before it is used as a container for say a string, it needs to point to memory sufficient for the string.

for example, this will work

word = malloc(80*sizeof(*word));
if(word)
{//success, continue

Similar to above, this:

char *word[n];   

extension is an an array of n pointers. Before any of the pointers can be used as a container for say some strings, each needs to point to its own memory location. Assuming the value n has been scanned in, this will work:

for(int i=0;i<n;i++)
{
    word[i] = malloc(80*sizeof(*word[i]));//80 for illustration
    if(!word[i])
    {//handle error...

Once the memory is allocated, the strings can be populated.
However, to ensure user input does not overflow the buffer defined by each instance of word, use a width specifier in the format string:

Change:

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

To:

scanf("%79s", words[i]);//note & is not needed as the symbol for a char array serves as the address
//      ^^    ^
ryyker
  • 22,849
  • 3
  • 43
  • 87