fix safety and correctness bugs in hostmot2 drivers

[sleepybishop]

Hello! This PR fixes some subtle safety and correctness bugs in the hostmot2 HAL drivers.

A few things this cleans up:

• Hardened socket send/receive checks in hm2_eth to ensure packet sizes match exactly, and added some bounds checking to prevent overflows.
• Plugged a firmware memory leak on failure in setsserial.c and made krealloc pointer handling safer across the board.
• Fixed some integer overflows when bit-shifting in pktuart.c.
• Fixed a copy-paste bug in the Three-Phase PWM config (tp_pwmgen.c) and an off-by-one bug in hm2_sserial_cleanup.

Please let me know if you have any feedback or see any issues with these fixes!

[grandixximo]

Hardware tested?

[sleepybishop]

Hardware tested?

Yes, but my own not mesa hardware. Most of my robots use a mix of artix and ecp5's. I do have an old cnc router with a 6i25 but that machine is currently buried under a pile of boxes.

[grandixximo]

Two things I would want resolved before this goes in.

The if (r < 0) => if (!r) conversion is not safe for every llio. Details inline on pktuart.c; short version is that hm2_spi.c returns negative errnos, and a negative value is truthy.

The sserial state machine hunks revisit #4144. Those six if (ret < 0) { ...; break; } additions are the fix proposed there. #4144 was closed without the behavior change, on the concern that smart-serial might start quitting with spurious errors.

The half that made that fix safe is also missing here: case 100 does not set written = param, so with the break in place a parameter whose write keeps failing stays dirty and is retried on every pass through the list, with an HM2_ERR each time. Today the code marches on after a failed wait, and the write is silently lost but never repeated.

@BsAtHome @andypugh you both have the context on #4144. Should these hunks land here with the case 100 half added, or go back to that issue?

[grandixximo]

This conversion is repeated about forty times across pktuart.c, uart.c and bspi.c, and it assumes every llio read/write returns 1 or 0. hm2_spi.c does not:

static int do_read(hm2_lowlevel_io_t *llio, rtapi_u32 addr, void *buffer, int size) {
    hm2_spi_t *this = (hm2_spi_t*) llio;
    int r = queue_read(llio, addr, buffer, size);
    if(r < 0) return r;
    return do_pending(this);
}

queue_read/queue_write return -EINVAL on a misaligned size, and do_pending() returns -errno when the SPI_IOC_MESSAGE ioctl fails. A negative value is truthy, so !r reads a genuine SPI failure as success; the r < 0 test it replaces caught exactly that case. Net effect: error detection for pktuart, uart and bspi disappears on Raspberry Pi hm2_spi boards.

r <= 0 covers both conventions. Worth auditing the other llio backends before settling on one.

[BsAtHome]

According to the llio interface, the read/write functions return 1 on success and 0 on failure. The hm2_spi driver is wrong. The hm2_rpspi driver is wrong too (only partially). The hm2_spix (and subordinate) driver does it correctly.

[BsAtHome]

@BsAtHome @andypugh you both have the context on #4144. Should these hunks land here with the case 100 half added, or go back to that issue?

I think it is necessary to split this PR and address the issue classes separately.

The fixes on the tests of llio write/read failure are important. But it must be consistent. The llio drivers must do the right thing too.
And then, a huge swat of the read/write calls never get checked at all. Implementing guards everywhere would not necessarily be a good thing because it is sometimes unclear what you can do to resolve the issue and most would bug the user at thread-cycle rates. So changing it all is not really an option.

I suggest to take out all the llio drivers' write/read issues (where the return value is checked, like pktuart) and fix the llio read/write in the hm2_spi/hm2_rpspi drivers as well. However, it must be said that both are superseded by the hm2_spix driver. There may be other llio drivers that need to be checked for the correct return values.

[BsAtHome]

Fixed the llio SPI drivers in #4489.

[sleepybishop]

Thanks. I removed the LLIO convention changes from this PR and defer to #4489 to handle the legacy SPI backends. I tried to reorganize the remaining fixes into focused commits, but agree splitting is also fair. Let me know what would make the most sense.

[BsAtHome]

Will need to do a deeper dive to see how a split would be best.

I'm especially wary about changes in sserial and the encoder stuff. That has also been a pain in porting to the getter/setter infrastructure. Any changes in there must be checked against running hardware. So these qualify for a very limited separate PR.

[grandixximo]

Before you dive into the split, is it worth getting PCW one pass over the branch first? Would one branch be less work to test than two or three separate PRs?

Results recorded against commit hashes would survive whatever split you land on, as long as the branch is not rebased before then.