4

I'm following along that tutorial, I'm running WindowsXP 32-bit with cygwin compiler, The tutorial asks me to run this code:

#include <stdio.h>
#include <ctype.h>

// forward declarations
int can_print_it(char ch);
void print_letters(char arg[]);

void print_arguments(int argc, char *argv[])
{
    int i = 0;

    for(i = 0; i < argc; i++) {
        print_letters(argv[i]);
    }
}

void print_letters(char arg[])
{
    int i = 0;

    for(i = 0; arg[i] != '\0'; i++) {
        char ch = arg[i];

        if(can_print_it(ch)) {
            printf("'%c' == %d ", ch, ch);
        }
    }

    printf("\n");
}

int can_print_it(char ch)
{
    return isalpha(ch) || isblank(ch);
}


int main(int argc, char *argv[])
{
    print_arguments(argc, argv);
    return 0;
}

But I keep running into this warning:

$ make ex14
cc -Wall -g    ex14.c   -o ex14
ex14.c: In function ?can_print_it?:
ex14.c:34:5: warning: array subscript has type ?char? [-Wchar-subscripts]
    return isalpha(ch) || isblank(ch);
    ^
ex14.c:34:5: warning: array subscript has type ?char? [-Wchar-subscripts]

I have tried searching around, while I have found a lot of threads talking about a similar error, none of their answers/solutions worked on my code, and I couldn't find a question related to Learn C The Hard Way exercise 14.

So what should I do to get rid of that warning?

Improve this question
13
  • Does the code work anyway? It's throwing a warning, not an error... (By the way, I'm amazed that there is a "learn-c-the-hard-way" tag :) Commented Jan 28, 2015 at 15:11
  • 1
    (unsigned char)ch . The standard mandates the passed value to both of those (isalpha and isblank) must be representable as unsigned char or the behavior is undefined. Commented Jan 28, 2015 at 15:12
  • @AjPerez, whoa my bad, It actually works, I still want to get rid of that warning though Commented Jan 28, 2015 at 15:13
  • I do not understand why the compiler considers char index as dangerous, but you can change can_print_it(char ch); to can_print_it(int ch); to fix it. Commented Jan 28, 2015 at 15:13
  • 1
    @iharob because they want to use heavier restriction than just printable. The goal of the poorly named can_print_it is to exclude all but alphabetic chars or spaces. Punctuation, digits, etc, are all excluded, where they would not be with isprint. Commented Jan 28, 2015 at 15:44

4 Answers 4

Reset to default
4

The reason for the warning is that char values are usually positive, but sometimes they may be negative; this may come unexpected to the programmer (especially because on some implementations char is always positive), and using a negative index for an array is obviously a bad thing.

isalpha does exactly that behind your back. You fix the warning by casting the char to int or by storing it in an int in the first place - this doesn't fix the problem though, because a negative char will be cast to a negative int which has exactly the same problem. You can fix the warning and the problem by casting the char to an unsigned char.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

Changing both of the functions to accept unsigned char's has worked
The compiler shouldn't warn for this. The default argument promotions ensure that the char is converted to an int through integer promotion. The original signedness of char for the given implementation is therefore completely irrelevant, as it will always be converted to a (signed) int. Furthermore, there are no arrays in the function can_print_it(). Therefore I believe the correct answer is "the compiler warning is broken".
3

So if we look at the gcc document for warnings which covers -Wchar-subscripts it says:

Warn if an array subscript has type char. This is a common cause of error, as programmers often forget that this type is signed on some machines. This warning is enabled by -Wall.

It is implementation defined whether char is signed or unsigned, if you use an unsigned char then the warning will go away.

We can see from the draft C99 standard that the arguments to functions in <ctype.h> are expected to be representable as unsigned char or EOF, from section 7.4 Character handling <ctype.h>:

In all cases the argument is an int, the value of which shall be representable as an unsigned char or shall equal the value of the macro EOF. If the argument has any other value, the behavior is undefined

So most likely isalpha and isblank are implemented as a macro which uses a lookup table and so this code would indeed be using a char to index an array.

Improve this answer

1 Comment

If isalpha etc are implemented as macros, the implementation (compiler) is broken, because making them macros would remove the default argument promotion of char to an int, which the programmer will expect to happen, since they are functions. And there are no arrays in the function can_print_it(), so the compiler should not give this warning.
0

The input type of isalpha and isblank is expected to be int, not char. That's the core issue. The rest are details of how those are implemented. If use the -E flag to just pre-process the code, the function can_print_it expands to:

int can_print_it(char ch)
{
    return (((__ctype_ptr__+sizeof(""[ch]))[(int)(ch)])&(01|02)) || __extension__ ({ __typeof__ (ch) __x = (ch); (((__ctype_ptr__+sizeof(""[__x]))[(int)(__x)])&0200) || (int) (__x) == '\t';});
}

As you can see, the input, ch, is used as an array index in the implementation of isalpha. A temporary, __x, which is of the same type as ch, is use as an array index in the implementation of isblank.

You can fix the warnings by:

  1. Changing the argument type of can_print_it to int.

    int can_print_it(int ch)
{
    return isalpha(ch) || isblank(ch);
}

  2. Creating a temporary variable in the function and using it to call isalpha and isblank.

    int can_print_it(char ch)
{
    int temp = ch;
    return isalpha(temp) || isblank(temp);
}
Improve this answer

11 Comments

Both would fix the warnings, but neither will fix the negative-index lookup when passed a high-ascii value, which is exactly what that warning is designed to address. The former (1) is closer to the metal, but to properly invoke it requires if(can_print_it((unsigned char)ch)) to ensure the conversion to int doesn't fall right back in to a negative index. (and btw, I did not downtick this).
@WhozCraig, If an int is accepted as a valid argument to isalpha and isblank, which can be a negative value in theory, it's not clear to me how gcc is protecting the use with this warning.
Because when int is passed it is not promoted (its already int). If said-int was negative (and it certainly can be, as the type is definitely signed), well, it sucks to be the guy passing a negative int. But char may be signed (usually is). It is (a) promoted to int, and (b) if signed and negative, will have said-sign extended during the promotion. So joe sixpack not knowing his char may actually be a signed-negative value will find out the hard way.
And btw, your second alternative would work if int temp = (unsigned char)ch; were the temporary, though I still think the first is more-correct, putting the onus on the caller to do the cast, just like all the standard ctype iswhatever functions.
@RSahu: On some systems the "é" character (for example) is represented by a char that is negative. The rules for isalpha etc specify that the value passed to them must be representable by an unsigned char (or EOF, but let's ignore that case for now), which negative numbers are not.
|
0

It would appear that your compiler is old or broken. GCC 4.9.1 compiles this code with no warnings/errors, given:

gcc test.c -std=c11 -pedantic-errors -Wall -Wextra

The compiler is obliged to implicitly promote your char to the int that the isalpha etc functions require. It will do this promotion no matter the implementation-defined signedness of char. The size of char should not matter either, since char is always a small integer type.

The only kind of warning I'd expect a compiler to give from this code is something along the lines of "implicit conversion from char to int".

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.