Compiler diagnostics reveal errors

[MitchBradley]

I'm trying to compile with the 2018-q4 toolchain. It's throwing some errors and warnings which look like coding errors. Namely:

./plan_arc.cpp: In function 'stat_t _compute_arc(bool)':
./plan_arc.cpp:346:57: warning: 'arc_time' may be used uninitialized in this function [-Wmaybe-uninitialized]
     float segments_for_minimum_time = _estimate_arc_time(arc_time) * (MICROSECONDS_PER_MINUTE / MIN_ARC_SEGMENT_USEC);

_estimate_arc_time() does not reference arc_time before setting it, so arc_time should be a local variable instead of a parameter.

./util.h:117:31: error: 'float abs(float)' conflicts with a previous declaration
 inline float abs(const float a) { return fabs(a); }

It appears that abs() isn't used anywhere, so this line can just be deleted

./gpio.cpp: In function 'uint8_t _io(index_t)':
./xio.h:137:19: error: ISO C++ forbids comparison between pointer and integer [-fpermissive]
 #define NUL (char)0x00      //  ASCII NUL char (0) (not "NULL" which is a pointer)
                   ^~~~
./gpio.cpp:532:23: note: in expansion of macro 'NUL'
     } while (++ptr != NUL);

I think it should be "++ptr != NULL".

Update: the above "fix" is wrong. The correct way is:

    for (ptr = cfgArray[index].token; *ptr; ptr++) {
        if (isdigit(*ptr)) {
            return (atoi(ptr)-1);   // need to reduce by 1 for internal 0-based arrays
        }
    }

../Motate/MotateProject/motate/Atmel_sam_common/SamUART.h: In member function 'char* Motate::_USARTHardware<uartPeriphe\
ralNumber>::getTXTransferPosition()':
../Motate/MotateProject/motate/Atmel_sam_common/SamUART.h:408:38: error: cannot convert 'bool' to 'char*' in return
             if (_tx_paused) { return false; }

I think it should be "return NULL;"

[MitchBradley]

A bit more compiler fussiness:

./report.cpp:562:29: warning: '}
   ' directive writing 2 bytes into a region of size between 1 and 11 [-Wformat-overflow=]
             sprintf(report, "{\"qr\":%u,\"qi\":%u,\"qo\":%u}\n", qr.buffers_available, qr.buffers_added,qr.buffers_rem\
oved);
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
`gmp./report.cpp:562:20: note: 'sprintf' output between 24 and 34 bytes into a destination of size 32
             sprintf(report, "{\"qr\":%d,\"qi\":%d,\"qo\":%d}\n", qr.buffers_available, qr.buffers_added,qr.buffers_rem\
oved);

Fixed this one by changing the report size from 32 to 34. I think the compiler is calculating that the static part of the string is 21 characters plus max 3 characters for uint8_t and 5 for each of the uint16_t's. Probably best to be safe and burn a couple of bytes on the stack to quench the warning.

./planner.cpp: In function 'void planner_init(mpPlanner_t*, mpPlannerRuntime_t*, mpBuf_t*, uint8_t)':
./planner.cpp:184:39: warning: 'void* memset(void*, int, size_t)' clearing an object of non-trivial type 'mpPlanner_t' \
{aka 'struct mpPlanner'}; use assignment or value-initialization instead [-Wclass-memaccess]
     memset(_mp, 0, sizeof(mpPlanner_t));    // clear all values, pointers and status

For this one I just said "_mp = {};". I hope that is right.

_Update: "mp = {};" does not work. I did not find a correct fix to suppress the warning so I kept the original code and lived with the warning.

[MitchBradley]

Update: The old compiler (the one that the unmodified Motate Tools uses) handles these changes just fine, so the changes are safe in that respect.

[giseburt]

Hi Mitch,

Thank you for looking into this. I'll look these over and make sure these get incorporated.

As I mentioned over in #401, if you look at dev-168-gquintic and follow to the motate branch it uses (a slightly older commit in sams70-port), then some of the things you mentioned have been dealt with. You are using an even newer compiler than that Motate is using, so there may be a few things that still need to be caught.

The newer Motate has also been cleaned up somewhat and has better separation of concerns. C++17 has added some really nice features that allow us to clean up some of the more hacky ways of handling Motate objects. It has to be non-breaking, so it's not as sweeping of change as some have asked for, but it's getting there. :)

[MitchBradley]

I see that the modified Motate does indeed resolve all of the constexpr problems with pointers.

I edited my comments above to reflect the code I am using successfully. The updates are in italics, prefaced with "Update:"