Re: [Bug-gnulib] addition: minmax.h

[Paul Eggert]

Bruno Haible <address@hidden> writes:

> /* Before we define the following symbols we get the <limits.h> file
>    since otherwise we get redefinitions on some systems.  */
> #include <limits.h>

I'd rather omit this #include.  On some systems, <limits.h> imports a
boatload of undesirable symbols.  For example, HP-UX 11i <limits.h>
declares gettimeofday and many other functions, some of them
HP-specific.  Furthermore, in many other systems some standard include
files other than <limits.h> define MIN and MAX (e.g., <inet/common.h>
on Solaris 7), so having <minmax.h> include <limits.h> doesn't really
solve the problem anyway.

<minmax.h> should #define only MIN and MAX.  It should be the caller's
responsibility to include <minmax.h> after any system file that could
define MIN or MAX.  Such a solution is not ideal, but it's better than
the "#include <limits.h>" solution.

Also, this is more of a documentation issue, but it would be helpful
to add a comment like this:

/* Caveat: MIN and MAX might not return the minimum and maximum of
   their two arguments, if the arguments have different types or have
   unusual floating-point values.  For example, on a typical
   two's-complement host with 32-bit int, 64-bit long long, and 64-bit
   double:

     MAX (-1, 2147483648) returns 4294967295.
     MAX (9007199254740992.0, 9007199254740993) returns 9007199254740992.0.
     MAX (NaN, 0.0) returns 0.0.
     MAX (+0.0, -0.0) returns -0.0.

   and in each case the answer is in some sense bogus.  */

This won't be an issue if the callers of MIN and MAX know what they're
doing, but not everyone knows of these gotchas and it's helpful to
write them down somewhere.

> #ifndef MAX
> # if __STDC__ && defined __GNUC__ && __GNUC__ >= 2
> #  define MAX(a,b) (__extension__                                         \
>                    ({__typeof__ (a) _a = (a);                             \
>                      __typeof__ (b) _b = (b);                             \
>                      _a > _b ? _a : _b;                                   \
>                     }))
> # else
> #  define MAX(a,b) ((a) > (b) ? (a) : (b))
> # endif
> #endif

A minor nit: the recommended GNU coding style is to put a
space after the commas.

More important, the semantics of this __extension__-based definition
differ between GCC and compilers, which can lead to nonportable code
if people develop using GCC and then deploy with other compilers.  Any
portable use of the __extension__-based MAX will have to avoid side
effects in MAX's arguments anyway.  Any portable and efficient use of
the __extension__-based MAX will also have to avoid expensive
computations in MAX's arguments.  So we might as well avoid the fancy
definition entirely, since it's not buying us any portability, and
it's not buying us much (if any) efficiency in portable and efficient
code.

In short, how about if we just replace the above with:

  #undef MAX
  #define MAX(a, b) ((a) > (b) ? (a) : (b))

This is what diffutils does, for example, and it works fine in
practice.  Coreutils uses the #undef approach in its lib/regex.c, and
also uses the following approach in its src/sys2.h:

  #ifndef MAX
  # define MAX(a, b) ((a) > (b) ? (a) : (b))
  #endif

This also works fine in practice.  I prefer the #undef approach,
though, since it "feels" safer not to rely on standard include files'
definition of MAX.

So here is a revised proposal:


/* MIN, MAX macros.
   Copyright (C) 1995, 1998, 2001, 2003 Free Software Foundation, Inc.

   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 2, or (at your option)
   any later version.

   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.

   You should have received a copy of the GNU General Public License
   along with this program; if not, write to the Free Software Foundation,
   Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */

#ifndef _MINMAX_H
#define _MINMAX_H

/* To avoid redefinition warnings from the compiler, include
   <minmax.h> after including any system file that might
   unconditionally define MIN or MAX.  For example, <limits.h>
   unconditionally defines MIN and MAX on some platforms, so you
   should include <minmax.h> after including <limits.h>.  */

#undef MAX
#undef MIN

/* Caveat: MIN and MAX might not return the minimum and maximum of
   their two arguments, if the arguments have different types or have
   unusual floating-point values.  For example, on a typical
   two's-complement host with 32-bit int, 64-bit long long, and 64-bit
   double:

     MAX (-1, 2147483648) returns 4294967295.
     MAX (9007199254740992.0, 9007199254740993) returns 9007199254740992.0.
     MAX (NaN, 0.0) returns 0.0.
     MAX (+0.0, -0.0) returns -0.0.

   and in each case the answer is in some sense bogus.  */

#define MIN(a, b) ((a) < (b) ? (a) : (b))
#define MAX(a, b) ((a) > (b) ? (a) : (b))

#endif /* _MINMAX_H */