Comment 2 for bug 711125

Yasufumi,

>Which option of gcc outputs the warnings "comparison between signed and unsigned"?

This is what MySQL maintainer mode uses: "-Wall -Wextra -Wunused -Wwrite-strings -Wno-strict-aliasing -Werror -Wno-unused-parameter" for C++ and "-Wall -Wextra -Wunused -Wwrite-strings -Wno-strict-aliasing -Werror -Wdeclaration-after-statement"

They are defined in config/ac-macros/maintainer.m4

>And why do you want to fix them?

I want to fix them, because I want to make sure every _new_ warning is addressed by developers. MySQL maintainer mode was specifically implemented for that purpose. As of 5.1.52 it is automatically turned on when you build --with-debug=full which is what I use for my development builds.

>"comparison between signed and unsigned"
>If compared correctly, no problem.

Sure, "correctly" is the keyword. Such comparisons is a frequent source of bugs. Consider:

  int a = -1;
  unsigned int b = 1;

  if (a > b)
    printf("Oops!\n");

And such bugs are typically hard to find too. Hence the compiler warning. It is just supposed to notify the developer that there _may_ be a problem with the code. You then have to take a look and decide whether the problem is real or not. But, as I wrote previously, if you have hundreds of such warnings in your build log, how will you notice the new one?

> And,
> "comparison of unsigned expression < 0 is always false"
> is for the future definition change. Even if the sign of the type was changed in mysqld, it works fine.
> It was remained intentionally, even if the pointed at the similar past bug report.

So if your code is correct about variables signedness (i.e. does not produce warnings), you _will_ get a warning whenever the signedness for some variable will change, right?

Let's look at the actual code producing those warnings:

---
buf/buf0buf.c: In function ‘_increment_page_get_statistics’:
buf/buf0buf.c:84: warning: comparison of unsigned expression < 0 is always false
---

This one is about variable introduced by _our_ patch. How can it change its signedness in the upstream development, if the upstream MySQL does not have it?

---
buf/buf0buf.c:86: warning: comparison is always false due to limited range of data type
---

Again, it's about the variable from our patch.

So what "future definition change" are you talking about?