Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory conflict: Multiple MIN ports access the same payloads_ring_buffer #48

Open
kinimodkoch opened this issue Jul 17, 2023 · 2 comments

Comments

@kinimodkoch
Copy link

There seems to be a bug if you use multiple MIN ports at the same time with the TRANSPORT_PROTOCOL option enabled.

There is only one ring buffer defined in min.c:

// Where the payload data of the frame FIFO is stored
uint8_t payloads_ring_buffer[TRANSPORT_FIFO_MAX_FRAME_DATA];

While all other port related data - including access indices for this buffer - are available for each port in

struct min_context

The effect is that each port overwrites buffer data of the other ports.

I suggest that
uint8_t payloads_ring_buffer[TRANSPORT_FIFO_MAX_FRAME_DATA];
is moved to
struct min_context
so that each port has its own ring buffer.

The required changes are:

  1. min.h - Add ring buffer in
    image

  2. min.c - Remove old ring buffer
    image

  3. min.c - make sure the two usages of the old buffer access the new buffer instead
    image
    image

@kinimodkoch
Copy link
Author

I just realized this topic was also discussed in #46, but I still think this is a bug.
The comment in #46 is that the application is not designed to work with multiple ports - but apart from this minor issue, the multi-port support is implemented well.
I also don't agree with the concern about the usage of the protocol in tiny controllers - the suggested fix has no memory impact if you instantiate only one port.

@kentindell
Copy link
Collaborator

The explanation is that I started to implement multiple ports but didn't complete. I'll accept a PR covering this, though.

0x3333 added a commit to 0x3333/min that referenced this issue Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants