-2

Why I am getting runtime error (SIGSEGV) on the following code:

#include<stdio.h>
#include<string.h>
int main()
{
    int t_line,count[10000],i;
    scanf("%d",&t_line);

    for(i=1;i<=t_line;i++)
    {
        fflush(stdin);
        gets(t);
        count[i]=(int)t[0]+(int)t[1]+(int)t[2];
    }

    for(i=1;i<=t_line;i++)
        printf("%d\n",count[i]);

    return 0;
}

I have also tried to solve this problem by initialized all the elements of array.

askmish
  • 6,464
  • 23
  • 42
Omar Faruq
  • 69
  • 5
  • 2
    Don't use `gets`. And what is `t`? And what's your input? And because you flush an input buffer: What's your platform? And in which line does it crash (use a debugger to find out)? – mafso Aug 02 '14 at 14:26
  • 1
    In C, you also count from 0 to < length, not 1 to <= length of buffer. – Iwillnotexist Idonotexist Aug 02 '14 at 14:29
  • 1
    Always check return value of `scanf`. In this case, if `scanf` fails, `t_line` will be left uninitialized, likely with large value, and the loops will go haywire. – hyde Aug 02 '14 at 14:36
  • 1
    Also, `fflush(stdin)` will likely not do what you expect it to do. What it does (if anything) is undefined, so better not use it. See for example http://stackoverflow.com/questions/2979209/using-fflushstdin – hyde Aug 02 '14 at 14:37

1 Answers1

1

I am wondering how the code compiled, without declaration of the variable t. But, still the only missing elemnt was: char t[your choice of size];. Apart from that 

#include<stdio.h>
//#include<string.h> No need of this header,a s you are not using any string functions
int main()
{
    int t_line,count[10000],i;
    char t[64];//you need to declare the variable before using it
    scanf("%d",&t_line);

    //Its safer if you check this
    if(t_line >= 10000)//if you use 0 and < t_line in for loop below then change the condition to:  if(t_line > 10000)
    {
       printf("ERROR: Limit exceeded. Not enough memory.\n");
       return 1;//or you could use exit(1); and #include <stdlib.h> 
    }

    for(i=1;i<=t_line;i++)//suggested: for(i=0;i<t_line;i++) 
    {
        //fflush(stdin);
        //gets(t);
        char *rc = fgets(t, sizeof(t), stdin);
        if(rc != NULL)
        {   t[strlen(t) - 1] = '\0';\\because fgets gets the \n  into the string too. This line makes fgets similar to gets, improving safety from overflow.
        }
        else
        {
             // handle fgets failed error
        }
        count[i]=(int)t[0]+(int)t[1]+(int)t[2];
    }

    for(i=1;i<=t_line;i++)//suggested: for(i=0;i<t_line;i++)
        printf("%d\n",count[i]);

    return 0;
}

Find the solution and suggested changes inline as code comments.

In C, its better to use indexes starting from 0, unless there is a specific requirement to use other values.

askmish
  • 6,464
  • 23
  • 42
  • 1
    I'd also remove the `fflush`, add return value checking for `scanf`, and replace `gets` with `fgets` while you're at it. – hyde Aug 02 '14 at 14:42
  • @hyde `if(t_line >= 10000)` will guard against failed `scanf` filling in garbage value in `t_line` and causing an undefined behavior. – askmish Aug 02 '14 at 14:48
  • Won't `for(i=1;i<=t_line;i++)` still crash at `i==10000`? – Floris Aug 02 '14 at 15:10
  • @askmish Inspecting value of `t_line` before it is initialized is undefined behavior in itself (unlikely to cause havoc on a PC, but still). Also, it doesn't catch the case of `t_line` happening to be <10000, and may read a "random" number of items on invalid input. Additionally, you should also check `t_line` is not negative (or change it to unsigned). – hyde Aug 02 '14 at 16:51
  • thanks it works nicely .In my program every word more precisely say t string need 3 character. I had forget that every string ended with null character so i therefore declared t[3] but it would be t[4]. – Omar Faruq Aug 03 '14 at 01:42