Removing the semicolons carries a small risk of silently changing behavior of existing code. For example the code:
a = MAX(b, c) - 5;
is two statements - "a = MAX(b, c);" and the declaration "-5;" which does nothing. This is probably NOT what the programmer intended (there's a bug) BUT the programmer may have worked around the bug in other ways to get the intended behaviour. So, removing the semicolon from the macro will silently change the result of the filter.
I've quickly checked where MIN and MAX are used in frei0r and I'm pretty sure it is safe to change. There is just one expression containing MIN/MAX without a following semicolon, in facebl0r.cpp:
//mask out-of-range valuescvInRangeS(obj->hsv, //sourcecvScalar(0, smin, MIN(vmin, vmax), 0), //lower boundcvScalar(180, 256, MAX(vmin, vmax) ,0), //upper boundobj->mask); //destination
I think the only way this can compile now is if OpenCV headers already define MIN and MAX. But I don't have OpenCV installed so I can't be sure what is happening here.
jaromil
commented
6 years ago
There are trailing semicolons on MIN and MAX macros in frei0r_math.h which could cause compiler errors or bugs. The problem is already discussed here: https://lists.dyne.org/lurker/message/20170822.153104.76f9159d.en.html
Removing the semicolons carries a small risk of silently changing behavior of existing code. For example the code:
a = MAX(b, c) - 5;is two statements - "a = MAX(b, c);" and the declaration "-5;" which does nothing. This is probably NOT what the programmer intended (there's a bug) BUT the programmer may have worked around the bug in other ways to get the intended behaviour. So, removing the semicolon from the macro will silently change the result of the filter.I've quickly checked where MIN and MAX are used in frei0r and I'm pretty sure it is safe to change. There is just one expression containing MIN/MAX without a following semicolon, in facebl0r.cpp:
//mask out-of-range valuescvInRangeS(obj->hsv, //sourcecvScalar(0, smin, MIN(vmin, vmax), 0), //lower boundcvScalar(180, 256, MAX(vmin, vmax) ,0), //upper boundobj->mask); //destinationI think the only way this can compile now is if OpenCV headers already define MIN and MAX. But I don't have OpenCV installed so I can't be sure what is happening here.