min() and max() problem in util.h

Development
1

Hi,

when fixing a 32 bit overflow in the HTTP analyzer, I cam across a
problem with the min() and max() function defined (as inline) in util.h
Their signature is int min (int,int), which fails for 64bit values and
which will also fails for comparing uints.

One could write a bunch of overloaded min/max function to handle all
kinds of argument types correctly. However, I don't like overloading
with too many too similar parameter types (bad experience with hard to
find bugs).

One way to solve this, would be to explicitly name the min/max functions
according to their type, e.g.,
   min_i32(), min_u32(), min_u64(), min_64()....

Or, I can just replace the min/max function with a macro. However, then
the problem is, that the expression that's passed to min/max will be
evaluated twice, which is a problem if it has side effects.....

Comments?
Ideas?

cu
Gregor

2

Or, I can just replace the min/max function with a macro. However, then
the problem is, that the expression that's passed to min/max will be
evaluated twice, which is a problem if it has side effects.....

GCC's typeof() extension can solve this, however, then we'd have
compiler dependent code :frowning:

(typeof - Wikipedia)

cu
Gregor

3

One could write a bunch of overloaded min/max function to handle all
kinds of argument types correctly.

Actually I think that's the best solution, and I don't really see
how it would cause hard to find bugs? min/max seems simple enough,
and as long as we don't provide versions with mixed argument types,
I think we should be fine. Or am I missing something nasty?

   min_i32(), min_u32(), min_u64(), min_64()....

That looks pretty ugly, in particualr when overloading can solve the
problem automatically.

Or, I can just replace the min/max function with a macro.

I'd expect that to actually generate more bugs than the overloading.

4

> One could write a bunch of overloaded min/max function to handle all
> kinds of argument types correctly.

Actually I think that's the best solution

That's my view too.

    Vern

5

One could write a bunch of overloaded min/max function to handle all
kinds of argument types correctly.

Actually I think that's the best solution, and I don't really see
how it would cause hard to find bugs? min/max seems simple enough,
and as long as we don't provide versions with mixed argument types,
I think we should be fine. Or am I missing something nasty?

Let's say we want to support:
   * int min(int,int)
   * int32_t min(int32_t, int32_t)
   * int64_t min(int64_t, int64_t)
   * (also for unsigned, alo for max)

Depending on the architecture the int one and int32 and int64 will have
the same signature and yield a compiler warning. We can probably just
omit the int version though, right? Since either int32 or int64 would be
a int (depending on the platform), right?

What happens, if I pass an int32 and an int64 to min()? Will I get a
compiler error or will the compiler to some coercion?

Also, what happens, if somebody tries to compare unsigned and signed?
Compiler error? (I know that gcc warns if you try to compare signed and
unsigned, but I'm not sure what it would do when it tries to find a
matching function signature)

Finally, if one wants to use a constant in a min/max call, one should
cast it to the right type, right? (If it's not casted it might cause
compiler errors, if the a constant has different width on different
platforms/compilers)

(*)

   min_i32(), min_u32(), min_u64(), min_64()....

That looks pretty ugly, in particualr when overloading can solve the
problem automatically.

yeah. that's pretty ugly

cu
Gregor

(*) Now that I think of it I can't reacall the details of my problems
with overloaded functions. I remember though that I had a problem with
an overloaded function that would pick the wrong function due to
something strange with the function signature and the types I passed to
the function....
However, I think that it involved polymorphism.....
Long story short: I can't really come up with a case in which
overloading min/max would be a problem, as long as we get compiler
errors when somebody tries to use unsupported types.

6

What happens, if I pass an int32 and an int64 to min()?

What happens if you do so to min_int64? To a MIN macro?

I think this thread is burning cycles on something that isn't particularly
broken to begin with.

    Vern

7

To finish this, I think we can trust the compiler here to pick the
right version, or to complain if it can't.

I've had problems with overloading picking the wrong version in the
past as well, but I think that always involved pointers of some
form; there's some implicit cast that C++ does in that context that
can cause trouble; don't remember the details either though. But I'm
pretty sure for ints we are fine.

Robin