Skip to content

Conversation

@JlnWntr
Copy link

@JlnWntr JlnWntr commented Apr 15, 2025

warning: variable length arrays in C++ are a Clang extension) extension] 1686 | Byte buffer[bufsize+16]; // pad for other struct members

I compiled this with g++ (clang, std c++17) on a recent macOS command line.

@keinstein
Copy link

I think this workaround should use vector instead of C-style arrays. This would use would automatically cleanup the allocated memory. As an alternative alloca and related functions could be used to put this array on the stack, so that it gets cleaned when the function is left in any way.

@JlnWntr JlnWntr reopened this Apr 23, 2025
@jonathon-bell
Copy link

First, thank you all for maintaining such a useful project.

I too see this warning using clang-20 on macOS. It's annoying, though harmless.

  • @keinstein I respectfully disagree that switching to an std::vector is the best fix here: its certainly would address the problem, but in my view is overkill because it introduces an essential use of the heap, which could possibly affect performance for what may be a performance critical section of code.

  • that leaves the stack: note that the function does not recurse, so the stack space consumed by placing the buffer on the stack is bounded and in the order of at most ~64k.

  • alloca also addresses the issue, but is a posix API, not standard C++. However this section of the code is already tied to the macOS API, so perhaps is the best fix.

  • another possibility, however, would be to simply ignore the size parameter and fix the buffer size at the maximum possible size of 64k+16. Yes, for many messages this is more than is needed, but....does that really matter? It seems a relatively small amount of stack memory to consume in the overall scheme of things, (unless perhaps compiling for an embedded system; does RtMidi support such platforms?)

  • yet another possibility is to simply disable the warning with a #pragma and leave the compiler to do it's thing.

Please let me know if I can be of help in putting together a PR - would be happy to do so.

Regards, Jonathon

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

Successfully merging this pull request may close these issues.

3 participants