Handle allocating on stack

[Livius90]

Can you change the handle struct declaration in your .h and .c files for all peripheral gpio, spi, led ... etc? For example if i move struct spi_handle { ... } from spi.c to spi.h i can able to use spi without any spi_new() allocation, so i can allocate it in stack, too in my main. Probably it should be useful and an optional alternative way for small and quick programs. Without this modification in headers the static allocation is not working because there are any declaration error in build.

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>

#include "spi.h"

int main(void) {
    spi_t spi;
    uint8_t buf[4] = { 0xaa, 0xbb, 0xcc, 0xdd };

    // not need in this case
    //spi = spi_new();

    /* Open spidev1.0 with mode 0 and max speed 1MHz */
    if (spi_open(&spi, "/dev/spidev1.0", 0, 1000000) < 0) {
        fprintf(stderr, "spi_open(): %s\n", spi_errmsg(&spi));
        exit(1);
    }

    /* Shift out and in 4 bytes */
    if (spi_transfer(&spi, buf, buf, sizeof(buf)) < 0) {
        fprintf(stderr, "spi_transfer(): %s\n", spi_errmsg(&spi));
        exit(1);
    }

    printf("shifted in: 0x%02x 0x%02x 0x%02x 0x%02x\n", buf[0], buf[1], buf[2], buf[3]);

    spi_close(&spi);

    // not need in this case
    //spi_free(spi);

    return 0;
}

[vsergeev]

I also like the simplicity of stack allocations, and c-periphery had this exact memory model in v1.x.x, but it was abandoned in v2.x.x because the GPIO handle state grew to be too complex when character device GPIO support was added (see https://github.com/vsergeev/c-periphery/blob/v2.2.2/src/gpio.c#L60). Using opaque pointers also has the benefit of a cleaner API and the ability to build c-periphery into a shared library with well-defined ABI compatibility and versioning. It was one of the main reasons the major version changed from 1 to 2.

Since c-periphery is always going to be running in a Linux environment and not a bare metal environment, I think a heap allocation in userspace for a couple of bytes is really not that big of a deal, and it makes it easier to share c-periphery handles out of the box in more complex software (i.e. without a user malloc).

There's no plans to go back to the old memory model for the reasons above. It might be possible to create a macro wrapper using alloca() for a stack allocated handle, e.g.:

size_t spi_handle_size(void);
spi_t *spi_init(spi_t *handle);
#define spi_new_alloca() spi_init(alloca(spi_handle_size()))
...

int main(void) {
    spi_t *spi = spi_new_alloca();
    ...
}

but I don't know if it's really worth adding that complexity to the API.

[Livius90]

You do not have to change your official concept, it could be just an alternative memory model which can used also, if you move that handle declarations in the header files. If the gpio, spi etc handle declarations would be in .h files it can be a user choose which one to use it, alloc it via spi_new() as in your example, or use it from stack as i mentioned in an example.

For example, i started a C++ wrapper class for your c-periphery spi and gpio and it would be nice to use it without calloc for easy and safe/quick using.

[vsergeev]

The issue is that the exposed handle structures become a part of the ABI and make versioning the shared library more difficult. If the handle structure is exposed and a field is added or removed, it would require a major version number change and break existing dynamically linked software. On the other hand, if the handle is opaque, modifying its fields wouldn't require a major version number change, since the library itself is creating it with the right members and size.

If we were confident the handle structure would never change, then maybe it would be OK to expose them. Providing a handle size getter and using alloca() is a compromise, but I suspect you probably want to declare handles as a POD instance within a class.

[Livius90]

This is my goal in my SPI class but now it works only with spi_t* spi and this->spi = spi_new().

class SPI {
private:
	spi_t spi;
	bool is_open;
public:
	SPI(string spi_path, unsigned int mode, uint32_t max_speed, string bit_order, uint8_t bits_per_word, uint8_t extra_flags);
	/* constructor, methods, etc */ 
}

[vsergeev]

I don't think I understand the underlying issue -- what's the problem in your example with using spi_t *spi; and spi_new()?

[Livius90]

I can allocate my SPI class on stack, and if i can allocate spi_t spi; in stack too, it could be very fast when the spi_transfer() function want to access it frequently.

[vsergeev]

I'm not certain there is a measurable concern here. The time spent in the ioctl() system call of spi_transfer() is going to dwarf any time spent accessing the fd in the spi_t structure on the heap or on the stack, or even the one-time call to malloc(). As you probably know, the SPI transfers are much much slower than any of these operations. If you're worried about the latency introduced by malloc() before the transfer begins, it's likely in the noise compared to any timing guarantees of I/O done from userspace Linux.