Bluetooth: GATT: The (mis)use of `ssize_t` for GATT callbacks

[Thalley]

Is your enhancement proposal related to a problem? Please describe. The return type ssize_t is used multiple places in the GATT API where the functions may return a value >= denoting bytes read/written, and a negative value in case of error.

Example:

    ssize_t (*read)(struct bt_conn *conn, const struct bt_gatt_attr *attr,
            void *buf, uint16_t len, uint16_t offset);

The definition of the POSIX type ssize_t is:

Used for a count of bytes or an error indication. According to POSIX, it shall be a signed integer type capable of storing values at least in the range [-1, SSIZE_MAX], and the implementation shall support one or more programming environments where the width of ssize_t is no greater than the width of the type long.

The positive value range is not much of a concern, but the GATT API often defines the negative return value range to be

BT_GATT_ERR() with a specific ATT error code.

The ATT error code range is 0x00 to 0xFF, and is made negative by the use of BT_GATT_ERR, so the return value range of the GATT API functions is thus [-255, SSIZE_MAX], which is of course outside the defined value range of the POSIX standard.

Describe the solution you'd like This issue has been created to create a discussion about this, and to determine if any change is needed.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context

Total definition from the man page:

ssize_t
              Include: <sys/types.h>.  Alternatively, <aio.h>,
              <monetary.h>, <mqueue.h>, <stdio.h>, <sys/msg.h>,
              <sys/socket.h>, <sys/uio.h>, or <unistd.h>.

              Used for a count of bytes or an error indication.
              According to POSIX, it shall be a signed integer type
              capable of storing values at least in the range [-1,
              SSIZE_MAX], and the implementation shall support one or
              more programming environments where the width of ssize_t
              is no greater than the width of the type long.

              Glibc and most other implementations provide a length
              modifier for ssize_t for the [printf(3)](https://man7.org/linux/man-pages/man3/printf.3.html) and the [scanf(3)](https://man7.org/linux/man-pages/man3/scanf.3.html)
              families of functions, which is z; resulting commonly in
              %zd or %zi for printing ssize_t values.  Although z works
              for ssize_t on most implementations, portable POSIX
              programs should avoid using it—for example, by converting
              the value to intmax_t and using its length modifier (j).

              Conforming to: POSIX.1-2001 and later.

              See also: [read(2)](https://man7.org/linux/man-pages/man2/read.2.html), [readlink(2)](https://man7.org/linux/man-pages/man2/readlink.2.html), [readv(2)](https://man7.org/linux/man-pages/man2/readv.2.html), [recv(2)](https://man7.org/linux/man-pages/man2/recv.2.html),
              [send(2)](https://man7.org/linux/man-pages/man2/send.2.html), [write(2)](https://man7.org/linux/man-pages/man2/write.2.html)

              See also the ptrdiff_t and size_t types in this page.

[Thalley]

Ping @alwa-nordic @Vudentz @jhedberg as code owners

[Thalley]

Additional note: It is not only GATT in Bluetooth that is returning more than -1, so this may be a general concern (or non-concern)

[alwa-nordic]

Option: Use int32_t, because it can represent error and max length. [-0xff, -1] and [0, uint16 max].

[jori-nordic]

I've seen this behavior in other places too, we could make a new type (not only for bluetooth) for that kind of return scheme.

[jhedberg]

It's really unfortunate that the standard sets this limit for ssize_t. It stems from the fact that standard libc APIs that return ssize_t use errno for reporting the exact error code, rather than the return value (which is just -1 in error cases). In practice ssize_t is always a typedef derived from a type which accepts a much bigger range of negative values than -1, so in that sense this is more of a theoretical rather than a real issue.

Option: Use int32_t, because it can represent error and max length. [-0xff, -1] and [0, uint16 max].

@alwa-nordic the existing convention is to use int for functions that return a negative (POSIX) error number. Let's not confuse people even more by adding a third convention (int32_t) into the mix.

I've seen this behavior in other places too, we could make a new type (not only for bluetooth) for that kind of return scheme.

There's an an existing issue that's tracking the idea of overhauling Zephyr error handling: #32213

[jhedberg]

So my default inclination would be to not change anything, but we can brainstorm some options. We could e.g. change the return type to int and follow the usual semantics for that, and then move the "number of bytes" information into a function parameter. We could either add a new size_t *bytes_read parameter, or update the existing uint16_t len parameter to be uint16_t *len where it contains the number of bytes requested on function entry and the actual number of bytes upon function return (and we might want to change the type to size_t *len in the same go if we'd do that).

[Thalley]

I'd rather not make any hasty changes to this either. As mentioned in this issue, this is not just a GATT issue, and https://github.com/zephyrproject-rtos/zephyr/issues/32213 does cover a more general approach.

It is also something that hasn't been urgent or an issue so far, so I'd like to wait and see what https://github.com/zephyrproject-rtos/zephyr/issues/32213 ends up being.

[theob-pro]

@Thalley should we close this issue, now that the issue you linked to has been closed as won´t do?

[Thalley]

It depends on whether we want to be POSIX compliant or not :)

The closed issue could have been an alternative solution for this, but otherwise changing the return value to int would be a simple solution, as it can contain all possible negative return values, as well as all positive lenght values (given the maximum size of 512 for ATT).

[zephyrbot]

Hi @jori-nordic, @jhedberg,

This issue, marked as an Enhancement, was opened a while ago and did not get any traction. It was just assigned to you based on the labels. If you don't consider yourself the right person to address this issue, please re-assing it to the right person.

Please take a moment to review if the issue is still relevant to the project. If it is, please provide feedback and direction on how to move forward. If it is not, has already been addressed, is a duplicate, or is no longer relevant, please close it with a short comment explaining the reason.

@Thalley you are also encouraged to help moving this issue forward by providing additional information and confirming this request/issue is still relevant to you.

Thanks!

[Thalley]

If we are changing the GATT API to support using individual ATT bearers, we can probably fix this issue at the same time.