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

Use fixed size arrays in case the compiler doesn't support C99 variable ... #7

Merged
merged 1 commit into from
Mar 13, 2015

Conversation

larskanis
Copy link
Collaborator

...length arrays - notably the MSVC compiler.

This is an alternative to https://bitbucket.org/ged/ruby-pg/pull-request/20 .

Note: The MSVC build environment is currently not tested on a regular base,
so is not fully supported.

…le length arrays.

This is notably the MSVC compiler.

Note: The MSVC build environment is currently not tested on a regular base,
so is not fully supported.
@lowjoel
Copy link

lowjoel commented Feb 7, 2015

How about testing for alloca instead, and only using the constant size array when both variable length arrays and alloca are not available?

GCC supports alloca, so while it isn't in POSIX, anywhere else where the extension is compiled using GCC alloca should be available.

@larskanis
Copy link
Collaborator Author

Variable length arrays are deallocated at the end of the block but alloca() at the end of the function (as you already pointed out). This makes it unusable as a simple preprocessor macro. We would need to use different code paths (#ifdefs or several macros), to implement both ways at all places which makes use of variable length arrays. The same is true for alternatives based on malloc/free or malloca/freea.

@lowjoel
Copy link

lowjoel commented Feb 8, 2015

yes, hence my PR on Bitbucket tended to move all variable declarations to the start of the function. The functionality is equivalent on both compilers supporting alloca and those which support VLAs since the sites where I changed to alloca the length of the array was an invariant throughout the invocation of the function.

I'm against using hardcoded lengths where possible. Indeed while MSVC isn't tested regularly, having this compiling fine might allow people to use in production, giving a potential buffer overflow vector, and a false sense of security.

There are multiple considerations here. I think it is more important to have correct support than having seemingly-correct support.

@larskanis
Copy link
Collaborator Author

@lowjoel the array size is checked - did you read the sources here ? Does it work with MSVC?

@lowjoel
Copy link

lowjoel commented Feb 8, 2015

Right, I glossed over that. If that's the case, then I have no complaints.

I have not built it yet. I'm not sure if it would work since there's a conditional in the variable initialiser.

@lowjoel
Copy link

lowjoel commented Feb 8, 2015

@larskanis thanks! Your branch compiles.

If I were to set up AppVeyor, how would the account be handled? Their config is a bit different from how Travis does it.

@larskanis
Copy link
Collaborator Author

Never used Appveyor myself, but I think something like forking ruby-pg (github or bitbucket), connecting to Appveyor, do necessary additions/changes to that fork and sending a pull request should work. The test itself should include a build and a test suite run. The PostgreSQL server and Ruby environment could be any supported version, since we do regular testing against oldest/newest supported versions on travis-ci already. @ged or I will add the Appveyor-connection to our accounts, to ensure it is tested on the MSVC platform.

@larskanis larskanis merged commit 94e9315 into ged:master Mar 13, 2015
@larskanis larskanis deleted the non-c99-compiler branch December 24, 2017 20:43
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.

2 participants