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

Answer 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;
}

Answer 2

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.

Answer 3

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;
}