Are there any plans to migrate to exact-width integer types?

[begincalendar]

Thank you to all contributors of this code base.

Unfortunately due to the lack of exact-width integer types (e.g. uint16_t), this library is difficult to adopt without potential further rework for each platform it is deployed on.

Are there any plans to adopt usage of <stdint.h> and migrate to exact-width integer types (so as to become more platform agnostic)?

[scaprile]

I've had no problem using this library, which is also true for many other developers. What is your issue, exactly ?

[begincalendar]

Mainly the following:

• Integrating with pre-existing code that uses fixed-width types may result in unexpected/undesirable overflows and/or undefined behaviour (e.g. int32_t being passed to this library as int on a platform where int is only 16-bits wide).
• More memory may be consumed than needed. E.g. ANSI C guarantees that int will be at least 16-bits wide, Let's say on platform X, int is 32-bits wide, but this library only needs the 16-bit width (in context Y). If the library had used int16_t (in context Y), there would be no wastage.
• While ANSI C provides some guarantees for numeric type widths, they are not as concrete and controlled as fixed-width integers. Control is relatively more important in the embedded world (than say in the web dev world).
• I would argue that code written with fixed-width types is easier to read/review.

This is not to single out embedded Paho, I think all embedded C/C++ code should use fixed-width types where feasible.

[begincalendar]

@scaprile I think we might have discovered at least one bug relating to this topic.

While the C standard guarantees that int is at least 16-bits wide, it appears as though the remaining length decoding function in this library assumes that int is at least 32-bits wide. On the platform we are using, int is only 16-bits wide.

[scaprile]

Yes, either both MQTTPacket_decode() and its child MQTTPacket_decodenb() (through the definition of struct MQTTtransport) assume int is at least 32-bits wide. In fact, now that you mention it, this assumption is also held (at least) in MQTTPacket_len(), as it assumes both its parameter and returned value are at least 32-bits wide. I can also assume that their callers carry the same assumption so this is a library-wide issue.

This issue seems to arise when packets are longer than 16383 bytes; and this is usually not common on 16-bit platforms. I'm curious to know if you are actually having an issue.

[begincalendar]

I'm curious to know if you are actually having an issue.

We are working on a proof of concept and the bug came up as part of our internal review of this library.

This issue seems to arise when packets are longer than 16383 bytes; and this is usually not common on 16-bit platforms.

For the proof of concept this will ring true for us, but for the long run we can't rely on chance to avoid the undefined behaviour of signed integer overflow (e.g. with multiple-pass streaming of a large enough amount of data).

[scaprile]

I guess you don't have 16K+ RAM available to hold 16K MQTT packets on your 16-bit device, so you most likely will need to discard these packets anyway. A packet with a 3-byte length field will overflow your int so I would catch this at MQTTPacket_decode() and MQTTPacket_decodenb() by setting

#define MAX_NO_OF_REMAINING_LENGTH_BYTES 2

Let's see what the maintainer thinks of this issue.

[begincalendar]

I guess you don't have 16K+ RAM available to hold 16K MQTT packets on your 16-bit device, so you most likely will need to discard these packets anyway.

That's correct. We don't have 16K+ RAM available, but in the long run we would need to provide the ability to stream that amount of data via MQTT. Given that this library (and I'd personally argue the MQTT protocol itself) assume that the length of an MQTT control packet is known up-front or that a control packet can always fit in memory, we would need to develop our own streaming APIs to suit our memory constraints.

A packet with a 3-byte length field will overflow your int so I would catch this at MQTTPacket_decode() and MQTTPacket_decodenb() by setting...

Thanks for the tip.

Let's see what the maintainer thinks of this issue.

That was my intention behind raising this decoding issue. While before it was more about a matter of opinion, now there is at least one concrete example of how fixed-width integer types would prevent bugs across any compatible platform.

[OlegHahm]

Maybe #238 is of any help for you.

[OlegHahm]

Btw the current code base don't even compile for a 16 bit platform.

[scaprile]

Btw the current code base don't even compile for a 16 bit platform.

Perhaps you should define "compile" and "16-bit platform", this is a fairly mature project and has been developed when C89 dominated the embedded C world. Many sections assume 32-bit ints, though.

wrt your ssize_t idea, it might work in your platform and not on others, where the code assumes a 32-bit int and you have a 16-bit size_t.

Anyway, the maintainer will decide eventually.

[OlegHahm]

Btw the current code base don't even compile for a 16 bit platform.

Perhaps you should define "compile" and "16-bit platform"

The definition of "compile" is very easy: take a C compiler and try to translate the source code into binaries. The term 16-bit platform is also very well defined: a computer architecture where the width of an integer or address has a width of 16 bit. If you try to take a compiler for such a platform the comparison in https://github.com/eclipse/paho.mqtt.embedded-c/blob/master/MQTTPacket/src/MQTTPacket.c#L93 will fail.

this is a fairly mature project and has been developed when C89 dominated the embedded C world.

Interesting, given the fact that the first MQTT specification has only been released in 1999. Even more interesting that back in these days 32 bit architectures were hardly available for microcontrollers.

wrt your ssize_t idea, it might work in your platform and not on others, where the code assumes a 32-bit int and you have a 16-bit size_t.

That's not correct. size_t is defined to always be big enough to store the "maximum size of a theoretically possible object of any type" according to ANSI C99 (compare https://en.cppreference.com/w/c/types/size_t or https://www.gnu.org/software/libc/manual/html_node/Important-Data-Types.html).

Anyway, the maintainer will decide eventually.

I'm under the impression there are no maintainers for this project any more given the fact that no pull request has been properly reviewed in the last couple of years - let alone be merged.

[scaprile]

Oh, I do love irony and sarcasm too. Unfortunately, I can't afford the time. Since you do know for sure, you'll have no issues. You can easily find the maintainer blog and ask him.

[OlegHahm]

Maybe you'll find the time if you refrain from wasting it for passive-aggresive and non-helpful comments. Good luck!