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

Initialization with not constant expression #36

Closed
SadE54 opened this issue Jun 3, 2017 · 6 comments
Closed

Initialization with not constant expression #36

SadE54 opened this issue Jun 3, 2017 · 6 comments

Comments

@SadE54
Copy link

SadE54 commented Jun 3, 2017

Hi,

in identify.c :
int row_average[q->w];

It seems some C99 extensions are used (and that are not supported by some compilers :-/
For example VSC++ 2015

@dlbeer
Copy link
Owner

dlbeer commented Jun 3, 2017

Yes, you will need a compiler that supports C99 to build it. If you're building on Windows then you might want to consider MinGW. You will likely also need to modify the Makefile, and you won't be able to build the demo program (which is Linux-only).

@kaworu
Copy link
Collaborator

kaworu commented Jun 6, 2017

@dlbeer any consideration for dropping VLA as their support is optional in C11?

@dlbeer
Copy link
Owner

dlbeer commented Jun 6, 2017 via email

@kaworu
Copy link
Collaborator

kaworu commented Jun 6, 2017

No idea about VC++ to be honest, I'm kind of hijacking this issue because I feel uneasy with VLA as they are seldom used in other projects and also are optional in C11.

alloca(3) isn't a big improvement as failure cannot be easily checked (e.g. the GNU man page say that on failure it can UB). I feel like a library should be as robust as possible, and neither VLA nor alloca(3) fit the bill. Allocating on the heap (i.e. malloc(3)) should be safer as we don't have any control on how deep the stack is, its limit, and q->w and will come with the usual cost of having to free the memory explicitely.

As for VC++, the situation may improve. While Microsoft has ignored C for long, they've ported OpenSSH to Windows and provide a Linux compatibility now. I think it has/will require them to support some C99 features (and maybe C11), so let's wait and see.

@dlbeer
Copy link
Owner

dlbeer commented Jun 6, 2017 via email

@kaworu
Copy link
Collaborator

kaworu commented Jun 7, 2017

Agreed it seems to be the way to go after a quick look. I'll happily work on a patch and PR.

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

3 participants