build fix for older gcc

[jsorg71]

No description provided.

[matt335672]

Hi Jay,

This is an interesting topic.

I had a chat with @metalefty recently on gitter about which C version we should be targeting. We didn't really know the answer and the coding standards page in the Wiki didn't help us either.

Whatever we decide should (in my opinion) be enforceable by automation (i.e. a Travis job).

I've just tried building the devel branch with the gcc -ansi flag. I think that's a non-starter as lots of useful (and safe) functions were added for C99 and later like snprintf().

I've then tried building with -Wdeclaration-after-statement which picks up the mods you've made in the PR. That's more along the lines of what you're trying to achieve I think. It also picks up on the following files which contain a few declarations after statements:-

• common/os_calls.c
• common/ssl_calls.c
• sesman/chansrv/chansrv.c
• sesman/chansrv/chansrv_fuse.c
• sesman/chansrv/devredir.c
• sesman/chansrv/rail.c
• sesman/session.c

We could either allow mixed declarations and statements, or ban them. If we go for the latter, there's a bit more to add to this PR, and we can also add a Travis job to pick up on any future ones.

I think that's really your decision. I don't have any strong feelings one way or the other.

What do you think @metalefty?

[metalefty]

Hi,

I also think it's Jays' decision but we should decide which C standards we target. I personally like C99 at least but I'm not in a strong position on this.

[jsorg71]

This PR is just about a compiler that used to work and recently does not. It's not about forcing any C version. Lets not make it too complected.

It's the variable define in the for () part that is causing the trouble. Not declaration after statement.

We(or I) have been purposely not stating what C standard xrdp officially support. I've always wanted to support them all :)

We can start a new discussion about this. Maybe something like officially support C99 but don't reject patches that expand compiles if someone wants as long as it doesn't slow code down or make it unreadable.

[jsorg71]

I think it got broke here a9ec1eb

[matt335672]

That makes sense. I'm also a fan of C99 as a basic compiler level but we can add exceptions to support other compilers.

Jay - are you able to say what the compiler is which doesn't like the variable declaration in the for loop? We can certainly add 'no variable declaration in for() statement' to the coding standard, but it would be nice to record why, so that we can justify that decision to contributors.

I'm happy for this one to be merged personally.

[metalefty]

Let's merge.

[jsorg71]

@matt335672 it is a SPARC machine I use for testing big endian compatibility in xrdp. It's the last version Debian that supports SPARC which is Debian 7. I think there are some options for newer distros that support SPARC or SPARC64 but I haven't updated yet.

[matt335672]

I remember getting Wheezy running on a SunBlade 150 at my last place. Those machines were really well engineered compared to the PCs of the day. In fact I think they've still got a few running as standalone X servers.

According to this link [debian.org] that should be running gcc 4.7.x. If I'm reading the release notes for 4.7 right that should be largely feature-complete for C99. So I'm a bit confused. When you get a mo', could you see that the output of gcc--version is on that machine?

Thanks.

[jsorg71]

jay@deb7sparc:~$ gcc --version
gcc (Debian 4.6.3-14) 4.6.3
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

[matt335672]

Thanks for that. I'll add a note to the coding standards regarding this issue, so we can push back on PRs using declarations-in-for.

[matt335672]

Change made to this page. Feel free to amend as appropriate.