pointer/integer type mismatch in a conditional expression

Question

So I have already had a look at other posts with similar headings but none of the suggested answers are working for me.

I have a function that calculates the frequency of a char in a string:

int frequency(char *s, char c) {
  int i;
  for (i=0; s[i]; s[i]==c ? i++ : s++);

  return i;
}

It works correctly but the compiler gives me the following error:

warning: pointer/integer type mismatch in conditional expression [enabled by default]

Could someone please explain why

Cheers

score 1 · Accepted Answer · answered Jun 02 '13 at 17:51

1

i++ is of type int, while the type of s++ is char *. In a conditional expression, you can't have two different types in the "then" and "else" branch, hence the warning.

Here the author of this code snippet was trying to be clever and short, but he simply got it wrong. I'd suggest rewriting this as

int frequency(const char *s, char c)
{
    int i;
    for (i = 0; s[i];)
         if s[i] == c
             i++;
         else
             s++;

    return i;
}

answered Jun 02 '13 at 17:51

as the code works, and does what i want it to do should i keep the same or should i change it to get rid of the warning (speaking in terms of style etc..) – cxzp Jun 02 '13 at 17:52
1

@cxzp "it works" is not a good measurement of code quality -- see edit. – Jun 02 '13 at 17:53
@H2C03 would using the original and changing the type of `int i` to `char i` work as well or would you suggest breaking it up? – cxzp Jun 02 '13 at 17:55
@cxzp If I say "rewrite it", I mean "rewrite it". And no, changing it to `char` is not good. **Think about it.** – Jun 02 '13 at 17:57

score 0 · Answer 2 · answered Jun 02 '13 at 17:56

Every expression must have a type. For this expression,

s[i]==c ? i++ : s++

it isn't clear what the type should be. i++ gives an integer, and s++ gives a char *. A char * is convertable to an int, which is basically the boolean value of whether the pointer is not null. So, by having the type of the expression be int, the compiler is able to make it work, but since this is very odd situation, you are getting the warning.

score 0 · Answer 3 · answered Jun 02 '13 at 22:20

The code as written is using the parameter s both as a pointer to a character, and as a character array indexed by i. The for loop is being used to iterate over the string, but the start of the string is being moved when a matching character is not found.

This is very clever code. Clever code is seldom a good thing.

"It works" because the result of the expression s[i]==c ? i++ : s++ is not being used. Each branch performs an action, returning a value of a different type. Neither of those values is used in another expression.

I typically use for loops for performing a defined number of iterations. In this instance I think a while loop is more appropriate.

Using `s` as a pointer

int frequency(char *s, char c) {
  int count = 0;

  while (*s != 0) {
    if (*s == c) { 
      count++; 
    }
    s++;
  }

  return count;
}

Using `s` as a character array

int frequency(char s[], char c) {
  int count = 0;
  int current = 0;

  while (s[current] != 0) {
    if (s[current] == c) { 
      count++; 
    }
    current++;
  }

  return count;
}

pointer/integer type mismatch in a conditional expression

3 Answers3

Using s as a pointer

Using s as a character array

Using `s` as a pointer

Using `s` as a character array