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

Avoid variable lengths arrays #368

Merged
merged 2 commits into from Apr 5, 2020
Merged

Avoid variable lengths arrays #368

merged 2 commits into from Apr 5, 2020

Conversation

cmb69
Copy link
Contributor

@cmb69 cmb69 commented Apr 4, 2020

Despite conforming to C99, variable length arrays are not supported by
MSVC, and maybe some other relevant compilers. Therefore we hard-code
the maximum required space.


This would solve the Windows PECL build failures. An alternative would be to use do_alloca() and friends (similar to php/php-src@9d31a42), but this appears to be overkill in this case.

Despite conforming to C99, variable length arrays are not supported by
MSVC, and maybe some other relevant compilers.  Therefore we hard-code
the maximum required space.
@lstrojny
Copy link
Collaborator

lstrojny commented Apr 5, 2020

@cmb69 can you check the travis output, it seems to segfault. Happy to roll a dot-one release very quickly to get the Windows build fixed.

For a longer term solution, I have opened #369

@cmb69
Copy link
Contributor Author

cmb69 commented Apr 5, 2020

Thanks for the ping! That segfault should be fixed now. I'll try to have a look at AppVeyor CI later today or tomorrow.

@lstrojny
Copy link
Collaborator

lstrojny commented Apr 5, 2020

@cmb69 thanks a lot!

And man it would be terrific to have a good Windows CI setup. Thank you very much already!

@lstrojny lstrojny merged commit c3c5156 into php-amqp:master Apr 5, 2020
@lstrojny
Copy link
Collaborator

lstrojny commented Apr 5, 2020

v1.10.1 is released now with the fix

@cmb69 cmb69 deleted the cmb/no-vla branch April 5, 2020 14:37
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.

None yet

2 participants