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.

Improve this question
5
  • printf("%s\n", words[i]); Commented Aug 27, 2020 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). Commented Aug 27, 2020 at 13:56
  • 1
    You have an array of pointers, but where do each pointer actually point? Commented Aug 27, 2020 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 *). Commented Aug 27, 2020 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. Commented Aug 27, 2020 at 15:22

3 Answers 3

Reset to default
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;
}
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

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]

Improve this answer

4 Comments

&words[i] is char** but words is not. It is an array of pointers and not a pointer to pointer.
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 **
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.
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.
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
//      ^^    ^
Improve this answer

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.