Why does my for loop go out of bounds?

Question

I have a program to check if a sentence read on the keyboard is a pangram.

But when displaying the results (in the last for-loop), it goes out of the expected bounds. If you uncomment the printf statement, you can see that even thought i<26 is set as the loop condition, it runs upto i = 27 and the array of size 26 has elements cache[26] = garbage value and cache[27] = 0. What is wrong with my last for loop ?

int main() {
    char* string = (char*)malloc(1024*sizeof(char));
    fgets(string, 1024, stdin);
    int i=0, cache[25]={0};
    while(string[i]!='\0' ){
        cache[(toupper(string[i])-'A')]++;
        i++;
    }
    for( i=0; i<26; i++){
       // printf("%d - %d\n",i,cache[i]);
        if(!cache[i]){
            printf("not pangram");
            return(0);
        }
    }
    printf("pangram");  
    return 0;
}

Change `cache[25]={0};` to `cache[26]={0};` and mind what @MikeCAT said. — haccks, Apr 17 '16 at 15:27
@paulpaul1076 My `cl.exe` from VC++2008 accepted code without casting. No compiling C code as C++! — MikeCAT, Apr 17 '16 at 15:28
`cache[(toupper(string[i])-'A')]` may be out-of-range when `string[i] == '\n'` — MikeCAT, Apr 17 '16 at 15:31
@MikeCAT http://cs626431.vk.me/v626431318/364d/NVe6RnNDyuQ.jpg — Pavel, Apr 17 '16 at 15:33
@MikeCAT i guess that's c++ then. Didn't notice you said "no compiling as C++" — Pavel, Apr 17 '16 at 15:34
@paulpaul1076 Your file extension is `cpp`, so you are using C++. — MikeCAT, Apr 17 '16 at 15:38
[Don't cast the result of `malloc` in C](http://stackoverflow.com/q/605845/995714) — phuclv, Apr 17 '16 at 16:00

Christophe · Accepted Answer · 2016-04-17T15:41:50.177

The problem is that your array is first too small for the 26 letters of the alphabet. It should be at least cache[26].

Then the following might go out of range for any non alphabetic chars (comma, space, etc...):

cache[(toupper(string[i])-'A')]++;

Going out of range will corrupt your memory (for example overwrite i or whatever else can happen when the behavior is undefined).

How to solve the issue?

You may consider protecting your cache increment:

 if (isalpha(string[i]))
     cache[(toupper(string[i])-'A')]++;

Note that some more exotic locale might consider some chars outside the range 'A'-'Z' as being alpha. So you may even want to be even stricter:

 int letter=toupper(string[i])-'A'; 
 if (letter>=0 && letter<26) 
     cache[(toupper(string[i])-'A')]++;

Why does my for loop go out of bounds?

1 Answers1