2

I need to read an array of n strings from 2 letters in each (e.g. n = 3, need to read something near "ab bf cs"). I use this code and get segmentation fault:

int n;
scanf("%d", &n);
char array[n][2];
char *tmp;
for (int i = 0; i < n; i++)
{
    scanf("%s", &tmp);
    strcpy(array[i], tmp);
}

Please help me to fix it!

Tim Tolparov
  • 79
  • 1
  • 9

3 Answers3

4

Problem 1

To store a string of length 2, you need a char array of size 3. Use:

char array[n][3];

Problem 2

You are using &tmp in the call to scanf. That is wrong on two accounts.

  1. The type is not appropriate. Type of &tmp is char**. scanf needs a char* with the %s format specifier.

  2. &tmp cannot hold a string.

You could use 

scanf("%s", tmp);

That will be OK from type point of view but it won't ok from run time point of view since tmp does not point to anything valid.

You can do away with tmp altogether and use:

scanf("%s", array[i]);

That entire block of code can be:

int n;
scanf("%d", &n);
char array[n][3];
for (int i = 0; i < n; i++)
{
    // Make sure you don't read more than 2 chars.
    scanf("%2s", array[i]);
}
R Sahu
  • 204,454
  • 14
  • 159
  • 270
3

There are a couple things you need to fix here.

  1. Remember that one "string" (char[]) always ends with a null terminating character \0. So, whenever you create a char[], you must leave enough space for that. For example:

    char str[3] = "foo"; looks like ['f', 'o', 'o'], but

    char str[4] = "foo"; looks like ['f', 'o', 'o', '\0'].

    Unexpected consequences can occur when using the former.

    This means that you should change char array[n][2] to char array[n][3].

  2. Remember that pointers are different than arrays. If you want to use a pointer, you must allocate memory for it, otherwise you will most likely seg fault.

    In your program, it's simple enough that you can probably get away with not using a pointer, especially since you know the the length of each string that you will be storing (2).

    You could change char *tmp to char tmp[3].

  3. You can actually refactor your code so you don't need the tmp variable at all. If you want, you can just store the word you read into the array directly. For example:

    scanf("%s", array[n]);

  4. If you are really insistent on using that tmp variable as a char *, then allocate memory for it, like so:

    char *tmp = (char*)malloc(3 * sizeof(char));

  5. As a sidenote, if you are adhering to the ISO C90 standards, you shouldn't intermix variable declarations in code, so in your loop where you have for(int i = 0...), you should declare int i; at the top of the file, and then assign it in the loop for(i = 0...).

Community
  • 1
  • 1
homersimpson
  • 4,124
  • 4
  • 29
  • 39
2

First, as user3121023 pointed out in the comments, char *tmp; creates a pointer without memory allocated to it. scanf() needs an array, and you need space for the terminating '\0' (which is probably the source of your segmentation fault). So you could try scanf("%s", array[i]);.

Second, it's a good idea to use "%2s" instead of "%s" so the user doesn't input 3 digits and overflow your buffer.

Here's one solution.

int n;
scanf("%d", &n);
char array[n][3];
for (int i = 0; i < n; i++)
{
    scanf("%2s", array[i]);
}
anonymoose
  • 819
  • 2
  • 11
  • 25