Support MSVC: remove use of VLAs

Bug #1942002 reported by Nathan Grenville-Hunt
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Fix Released

Bug Description

Hi Paul,

I'm using your (excellent) library in a project under Windows/MSVC. The library builds and works without issues except for two places in which variable length stack based arrays (VLAs) are present. Microsft have confirmed that no version of MSVC will ever support VLAs, so I submit a patch to remove their use.

Rather than replace the arrays with some other stack allocated memory (with alloca or similar) I opted to add two new dynamically allocated buffers to the VTermState object, both of which are allocated on first use and resized as required. I wasn't sure about the best naming to use (you'll be the judge), but I have tried to keep the code style in keeping with the rest of the project as far as I could (I'm no C programmer).

Please take a look at the patch (attached), which is based on r783. Also, please advise if you would prefer that I submit this patch some other way (I'm not particularly familiar with launchpad).

Thanks again,

Revision history for this message
Nathan Grenville-Hunt (ngrenvillehunt) wrote :
Revision history for this message
Paul "LeoNerd" Evans (leonerd) wrote :

Yes; it turns out the VLAs are problematic for a few reasons. The large unbounded one sometimes grows too big for the stack as well.

Performing a dynamic re-allocation during runtime is not possible; a main design point about libvterm is that it only allocates on creation or resize, and not during normal runtime.

Instead I've solved this by using the tmpbuffer for the main decoding buffer (and defaulting it a much larger 4096 byte size), and simply putting the recombine chars on the stack as a fixed-size array, because we only support up to 6 chars per cell anyway. That's small enough it can live on the stack.

That's now in source as of -r795

Changed in libvterm:
status: New → Fix Committed
Changed in libvterm:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Related questions

Remote bug watches

Bug watches keep track of this bug in other bug trackers.