Bugzilla – Bug 1793
Various errors by MacOS compiler
Last modified: 2013-11-15 05:36:19 UTC
Created attachment 1701 [details] bug fixes Even though I don't have clang (yet) on this Mac... 1) NS_ASSERT should be used consistently. E.g., if there is already a check that a variable is between two values, NS_ASSERT shouldn't be used to check that value/2 is in the (half) range. 2) isnan is a std:: function 3) do not check that an uint is greater than 0... it is for sure. As usual, MacOS is quite sensible to this sort of stuff. Attached the patch fixing the errors. By the way, I don't have clang here but the latest patch did include some defined-but-unused member variables in the LTE examples, so more errors on clang.
Tommaso, can you please clarify which issues give compilation problems with Mac and which not? I understand 2) and 3), but I do not agree with 1) - if you don't like assertions, you can always compile with -d optimized ;-)
(In reply to Nicola Baldo from comment #1) > Tommaso, can you please clarify which issues give compilation problems with > Mac and which not? I understand 2) and 3), but I do not agree with 1) - if > you don't like assertions, you can always compile with -d optimized ;-) Thanks, but I like my code in debug state... since I'm mostly fighting bugs (a lost battle, if you want my opinion). However you're right, I was a bit too harsh with the asserts. Here it is a 2nd patch, only covering what's strictly necessary. Asserts checking if an unsigned int is greater than 0 are removed. On the other hand, I'm still puzzled by code like this: int8_t EutranMeasurementMapping::ActualA3Offset2IeValue (double a3OffsetDb) { if ((a3OffsetDb < -15.0) || (a3OffsetDb > 15.0)) { NS_FATAL_ERROR ("The value " << a3OffsetDb << " is out of the allowed range (-15..15) dB" << " for A3 Offset"); } uint8_t ieValue = lround (a3OffsetDb * 2.0); NS_ASSERT (ieValue <= 30); return ieValue; } Since a3OffsetDb is less than 15.0, and ieValue is a3OffsetDb*2, ieValue is less than 30 for sure. Unless there something horrible with lround. In my opinion those type of tests are superfluous. Cheers, T.
Created attachment 1710 [details] Bug fixes (minimal)
It turns out that, while we were wasting time in philosophical discussions, Peter just went ahead and fixed it in changeset f206fb584abf :-) Anyway, thanks Tommaso for the updated patch. I integrated it with Peter's change (1 line killed). As for EutranMeasurementMapping::ActualA3Offset2IeValue(), it was actually a bug, a int8_t was to be used instead of an uint8_t. You see that even superfluous assertions turn out to be valuable.... changeset: 10413:3ea02a8923ff tag: tip user: Nicola Baldo <nbaldo@cttc.es> date: Fri Nov 15 11:20:38 2013 +0100 summary: fixed Bug 1793 - Various errors by MacOS compiler