1

I have written the following C code to get in a list of strings from the user. But the stored strings are giving out weird values.

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


#define MAX_STRING_LENGTH 50


void readInStrings(char* arr[],int n)
{
  int i=0;
  char line[MAX_STRING_LENGTH];

  for(i=0;i<n;i++){
    arr[i]=malloc(MAX_STRING_LENGTH);
    printf("Enter another string : ");
    scanf("%s",&arr[i]);
    //fgets(&arr[i],MAX_STRING_LENGTH,stdin);
  }



  printf("Strings read in correctly.... \n");

  printf("Displaying out all the strings:   \n");
  for(i=0;i<n;i++){
    printf("%s\n",&arr[i]);
  }
}



void testStringInputs()
{
  printf("Enter the number of entries : ");
  int n;
  scanf("%d",&n);

  char* strings[n];
  readInStrings(strings,n);
}

Input Sample:

Enter the number of entries : 3
Enter another string : Alladin
Enter another string : Barack Obama
Enter another string : Strings read in correctly....
Displaying out all the strings:
AllaBaraObama
BaraObama
Obama

Problems: 1) Why is one string not taken in as input at all?
2) Why are the displayed strings scrambled like that?

The problem is the same if I use gets() or fgets() in place of scanf().

Arjun J Rao
  • 925
  • 1
  • 10
  • 25
  • 1
    fgets is a much safer option if you make the correction that's given in the answer. You can easily overrun the buffer with scanf(and %s) or gets. – jonsca Mar 19 '11 at 05:04
  • If I don't use the &arr[i] in the printf statement, the program is crashing. And the problem remains the same even with fgets(), the strings come out scrambled. And one iteration of the input for loop is also skipped. – Arjun J Rao Mar 19 '11 at 05:15

3 Answers3

4

arr[i] is already a pointer, you don't need the &

Martin Beckett
  • 94,801
  • 28
  • 188
  • 263
1

Its better to use an array of arrays(two dimensional) instead of array of pointers. I had a tough time correcting your code. So I changed the code to this

#include <stdio.h>
#include <stdlib.h>
#define MAX_STRING_LENGTH 50
void readInStrings(char (*arr)[MAX_STRING_LENGTH],int n)
{
  int i;
  for(i = 0 ; i< n+1; ++i)
   fgets(*(arr+i),MAX_STRING_LENGTH,stdin);
  printf("Strings read in correctly.... \n");
  printf("Displaying out all the strings:   \n");
  for(i=0;i< n+1;i++){
    printf("%s",arr[i]);
  }
}
int main()
{
  printf("Enter the number of entries : ");
  int n;
  scanf("%d",&n);
  char strings[n][MAX_STRING_LENGTH];
  readInStrings(strings,n);
  return 0;
}
Prasoon Saurav
  • 91,295
  • 49
  • 239
  • 345
  • But an array of strings IS an array of pointers, isn't it? I knew that your solution was one way of doing it, but I wanted to be... conceptually clean, if you may. – Arjun J Rao Mar 19 '11 at 06:37
1

Removing the & (as the first answerer noted) in scanf("%s",&arr[i]); and in printf("%s\n",&arr[i]); did the trick for me. Also, note if you compiled with warnings at their highest, your compiler would have told you right away that the & was misplaced.

jonsca
  • 10,218
  • 26
  • 54
  • 62
  • Could you tell me why is one iteration of fgets() skipped when I use it? The scanf() works fine. – Arjun J Rao Mar 19 '11 at 06:38
  • 1
    Put in a `getchar();` immediately after `scanf("%d",&n);` in `testStringInputs()`. When you are typing "3 " 3 is being stored in n, but there's an excess '\n' in the input stream. The first fgets in the function was picking that up as a line of input. – jonsca Mar 19 '11 at 06:46
  • Yea. That worked. Thanks. Is there a place where I can learn to avoid more of these pitfalls in input/output in C? A book, or a website? – Arjun J Rao Mar 19 '11 at 07:07
  • For this particular problem [this](http://faq.cprogramming.com/cgi-bin/smartfaq.cgi?answer=1049157810&id=1043284351) is a good link, but in general [this](http://c-faq.com/) is a good reference. There's a book on C pitfalls by Andrew Koenig, but I've never read it. I get a lot of info from places such as S.O. also. – jonsca Mar 19 '11 at 07:16