Created attachment 169728 [details] Fix type for abs() calls in net/opal During the exp-run in bug 208158, it was found that net/opal gives errors with libc++ 3.8.0 [1]: ../common/mpi.cxx:135:18: error: call to 'abs' is ambiguous distance = ( abs(MPIs[i].width - desiredWidth ) * ^~~ This is because abs() is being called with unsigned arguments. Fix this by casting the arguments to the appropriate signed type. [1] http://package18.nyi.freebsd.org/data/headamd64PR208158-default/2016-03-22_18h30m05s/logs/errors/opal-3.10.10_9.log
It is probably safer to cast each of the arguments to the subtraction to int so that the subtraction produces a signed result rather than casting the result of the subtraction. The latter should work in this case, but might not if the cast is to a wider type.
It doesn't matter too much either way, since all the arguments and the end result (distance) are unsigned. There will always be some cases where that distance is calculated incorrectly, if regular int in combination with abs() are used. The 'more correct' fix would be to define a separate function to calculate the distance between two unsigned values, and use that instead. But that is more churn, for very little gain.
E.g. something like this: --- plugins/video/common/mpi.cxx.orig 2013-02-20 02:18:05 UTC +++ plugins/video/common/mpi.cxx @@ -118,6 +118,11 @@ unsigned MPIList::getSupportedMPI( unsig return PLUGINCODEC_MPI_DISABLED; } +static inline unsigned udiff(unsigned u, unsigned v) +{ + return u >= v ? u - v : v - u; +} + bool MPIList::getNegotiatedMPI( unsigned* width, unsigned* height, unsigned* _frameTime) { unsigned i = 0; @@ -132,8 +137,8 @@ bool MPIList::getNegotiatedMPI( unsigned // to the desired one or matches it for (i=0; i < MPIs.size(); i++) { // we square the value in order to get absolute distances - distance = ( abs(MPIs[i].width - desiredWidth ) * - abs(MPIs[i].height - desiredHeight) ); + distance = ( udiff(MPIs[i].width, desiredWidth ) * + udiff(MPIs[i].height, desiredHeight) ); if (distance < minDistance) { minDistance = distance;
It would matter if you were casting to long to avoid overflow issues (long)(u1 - u2) != (long)u1 - (long)u2 if sizeof(long) > sizeof(int) Your code in #3 is the best way, though.
A commit references this bug: Author: truckman Date: Mon Jun 6 23:55:08 UTC 2016 New revision: 416491 URL: https://svnweb.freebsd.org/changeset/ports/416491 Log: Fix type for abs() calls in net/opal During the exp-run in bug 208158, it was found that net/opal gives errors with libc++ 3.8.0 [1]: ../common/mpi.cxx:135:18: error: call to 'abs' is ambiguous distance = ( abs(MPIs[i].width - desiredWidth ) * ^~~ This is because abs() is being called with unsigned arguments. Fix this by casting the arguments to the appropriate signed type. This mimics what happens with older libraries where the only version of abs() was the one in <stdlib.h>, which is prototyped: int abs(int) Correct functioning of this expression relies on how integer overflow actually behaves, which is actually undefined in the C++ standard. PR: 209077 Submitted by: dim Changes: head/net/opal/files/patch-plugins_video_common_mpi.cxx