gets() problem in C

Question

I wrote the following code:

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

#define SIZE 128

int main ()

{
    char mychar , string [SIZE];
    int i;
    int const count =0 ;    

    printf ("Please enter your string: \n\n");
    fgets (string, SIZE, stdin);

    printf ("Please enter char to find: ");
    mychar = getchar();

    for (i=0 ; (string[i] == '\0') ; i++ )
        if ( string[i]  == mychar )
            count++;

    printf ("The char %c appears %d times" ,mychar ,count);

    return 0;
}

The problem is that the gcc gives me an error for the 'int const count': " increment of read-only variable ‘count’".

What seems to be wrong ?

Thank !

Always use **`fgets()`** instead of `gets`. Also there are lots of stuff to fix. First, you shouldnt use standard library functions for creating user interface. Standard library is really **not designed** for that. Instead you should use **curses** library or write the program to **accept [arguments](http://www.gnu.org/s/libc/manual/html_node/Program-Arguments.html#Program-Arguments)** as input. — Athabaska Dick, Apr 12 '11 at 08:21
Drop the `const` from the declaration of `count`. Declaring something `const` means you do not intend for the value of that variable to change. — John Bode, Apr 12 '11 at 15:38

score 3 · Accepted Answer · edited May 23 '17 at 09:58

3

Try using fgets instead as:

fgets (string, SIZE, stdin);

Why gets is unsafe, has been answered several times on SO. You can see this.

edited May 23 '17 at 09:58

Community

1
1

answered Apr 12 '11 at 08:17

codaddict

445,704
82
492
529

This. `scanf()` is not a suitable replacement for `gets()`. – Greg Hewgill Apr 12 '11 at 08:20
How do I keep count value after for is ended ? I tried 'const int' – Batman Apr 12 '11 at 08:37

Christian Kastberg · Answer 2 · 2012-05-22T09:34:55.423

To make this example work you should also change the line:

if(*string == mychar) ++count;

into

if(string[i] == mychar) ++count;

Full working example is now:

#include <stdio.h>

int main(int artc, char *argv[])
{
/* arguments are strings so assign only the first characte of the
 * third argument string. Remember that the first argument ( argv[0] ) 
 * is the name of the program. 
 */
char  mychar = argv[2][0];
char *string = argv[1];
int i, count = 0;

/* count the occurences of the given character */
for (i=0 ; (string[i] != '\0') ; i++ )
    if(string[i] == mychar) ++count;

printf("The char ‘%c’ appears %d times in the sentence: %s\n", mychar, count, string);

return 0;
}

Athabaska Dick · Answer 3 · 2011-04-12T09:15:02.307

Always use fgets() instead of gets. Also there are lots of stuff to fix. You shouldnt use standard library functions for creating user interface. Standard library is really not designed for that. Instead you should use curses library or something similar. You could also write the program to accept arguments as input.

Short example of proper use of the standard library. This version does not have any error checking so it assumes that user input is correct.

#include <stdio.h>

int main(int artc, char *argv[])
{
    /* arguments are strings so assign only the first characte of the
     * third argument string. Remember that the first argument ( argv[0] ) 
     * is the name of the program. 
     */
    char  mychar = argv[2][0];
    char *string = argv[1];
    int i, count = 0;

    /* count the occurences of the given character */
    for(; *string != '\0'; ++string)
        if(*string == mychar) ++count;

    printf("The char ‘%c’ appears %d times.\n", mychar, count);

    return 0;
}

Usage: ./count "Hello, World!" l

Output: The char ‘l’ appears 3 times.

EDIT: As for the original code. Change == to !=.

for (i=0 ; (string[i] == '\0') ; i++ )

to:

for (i=0 ; (string[i] != '\0') ; i++ )

The comparison was wrong.

I don't want to use the main's args[]. But the fgets() , getchar() fixed code, steal gives me 0 value for 'count', and a 'const int count' declaration only gives "increment of read-only variable ‘count’ ". — Batman, Apr 12 '11 at 08:53
Well thats just plain wrong. You cannot use `const` variables because they are only read only as the compiler warns you. The original code has still some problems. Change `==` to `!=` in the for loop and the counting should work. — Athabaska Dick, Apr 12 '11 at 09:15

score 0 · Answer 4 · edited Oct 21 '12 at 14:32

This will do:

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

#define SIZE 128

int main()
{
  char mychar, string[SIZE];
  int i;
  int count=0;    

  printf("Please enter your string: ");
  fgets(string, SIZE, stdin);

  printf("Please enter char to find: ");
  mychar = getchar();

  for (i = 0; (string[i] != '\0'); i++)
    if (string[i] == mychar) ++count;

  printf("The char %c appears %d times in the sentence: %s" ,mychar ,count, string);

  return 0;
}

score 0 · Answer 5 · answered Apr 12 '11 at 08:16

0

Consider replace with "scanf( "%s", &string)" instead.

answered Apr 12 '11 at 08:16

Artem Barger

40,769
9
59
81

This is equally unsafe as `gets`. – Fred Foo Apr 12 '11 at 09:13

score 0 · Answer 6 · answered Apr 12 '11 at 08:18

0

gets is dangerous because it lets you read in more data than you've allocated space for, you can use fgets which specifies how many characters it is going to read in and stops if it finds a newline.

answered Apr 12 '11 at 08:18

gizgok

7,303
21
79
124

score 0 · Answer 7 · answered Apr 12 '11 at 08:50

0

gets is dangerous because it can take in more data than the size of the variable. Thereby exposing the system to attacks and compromising security. fgets should be used as it limits no. of characters to be read.

answered Apr 12 '11 at 08:50

Khemka

108
2
9

gets() problem in C

7 Answers7