[1/3] android: piglit-vbo.cpp compile error

Android is a bit more strict when it builds. Missing was ctype.h.
It doesn't break Linux.

Signed-off-by: Tom Gall <tom.gall@linaro.org>
---
 tests/util/piglit-vbo.cpp |    1 +
 1 file changed, 1 insertion(+)

These changes seem innocuous enough.

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

On 01/28/2013 10:30 AM, Tom Gall wrote:
> Android is a bit more strict when it builds. Missing was ctype.h.
> It doesn't break Linux.
>
> Signed-off-by: Tom Gall <tom.gall@linaro.org>
> ---
>   tests/util/piglit-vbo.cpp |    1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/tests/util/piglit-vbo.cpp b/tests/util/piglit-vbo.cpp
> index 4900931..b2e2e63 100644
> --- a/tests/util/piglit-vbo.cpp
> +++ b/tests/util/piglit-vbo.cpp
> @@ -93,6 +93,7 @@
>   #include <string>
>   #include <vector>
>   #include <errno.h>
> +#include <ctype.h>
>
>   #include "piglit-util-gl-common.h"
>   #include "piglit-vbo.h"
>

Thanks. These are reviewed-by me and now committed.

By the way, which Piglit branch are you using for Android?

I'm using the "android" branch on:

git://git.linaro.org/people/tomgall/piglit.git

It's a WIP however what is there builds as long as you put it into
external/piglit as well as have waffle already built. It uses
Android.mk files as compared to Adrian's earlier patches that he
posted but the piglit-framework-gl code for android is his. The
Android.mk file is split up tho it could be all put together into one
toplevel Android.mk. Feedback on that design choice as well as the
rest would be most welcome.

On Mon, Jan 28, 2013 at 5:14 PM, Chad Versace
<chad.versace@linux.intel.com> wrote:
> Thanks. These are reviewed-by me and now committed.
>
> By the way, which Piglit branch are you using for Android?

On Mon, Jan 28, 2013 at 2:09 PM, Ian Romanick <idr@freedesktop.org> wrote:
> These changes seem innocuous enough.
>
> Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

They are... but 2/3 and 3/3 are adding unreachable return statements
after switches with a default case that asserts. Kind of bogus.

Sure, but tell that to the compiler.

It is bad form to not have a return at the end of a function which is
supposed to return something.  The other way perhaps is just knock out
the default case and return that. Either way.

On Mon, Jan 28, 2013 at 7:01 PM, Matt Turner <mattst88@gmail.com> wrote:
> On Mon, Jan 28, 2013 at 2:09 PM, Ian Romanick <idr@freedesktop.org> wrote:
>> These changes seem innocuous enough.
>>
>> Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
>
> They are... but 2/3 and 3/3 are adding unreachable return statements
> after switches with a default case that asserts. Kind of bogus.

On Mon, Jan 28, 2013 at 5:11 PM, Tom Gall <tom.gall@linaro.org> wrote:
> Sure, but tell that to the compiler.
>
> It is bad form to not have a return at the end of a function which is
> supposed to return something.  The other way perhaps is just knock out
> the default case and return that. Either way.

% cat t.cpp
#include <assert.h>

int f(int n) {
	switch (n) {
	case 1:
		return 1;
	default:
		assert(!"FAIL");
	}
}

% gcc -O2 -Wall -pedantic -Wreturn-type -Wno-return-type -c t.cpp

<no output>

What flags do I have to use to get an error or warning? I tried with
an without optimization, C and C++, and gcc versions 4.5.4, 4.6.3, and
4.7.2.

arm-linux-androideabi-gcc (GCC) 4.6.x-google 20120106 (prerelease)

external/piglit/tests/util/piglit_ktx.c: In function
'target_to_texture_binding':
external/piglit/tests/util/piglit_ktx.c:761:1: error: control reaches
end of non-void function [-Werror=return-type]

On Mon, Jan 28, 2013 at 7:18 PM, Matt Turner <mattst88@gmail.com> wrote:
> On Mon, Jan 28, 2013 at 5:11 PM, Tom Gall <tom.gall@linaro.org> wrote:
>> Sure, but tell that to the compiler.
>>
>> It is bad form to not have a return at the end of a function which is
>> supposed to return something.  The other way perhaps is just knock out
>> the default case and return that. Either way.
>
> % cat t.cpp
> #include <assert.h>
>
> int f(int n) {
>         switch (n) {
>         case 1:
>                 return 1;
>         default:
>                 assert(!"FAIL");
>         }
> }
>
> % gcc -O2 -Wall -pedantic -Wreturn-type -Wno-return-type -c t.cpp
>
> <no output>
>
> What flags do I have to use to get an error or warning? I tried with
> an without optimization, C and C++, and gcc versions 4.5.4, 4.6.3, and
> 4.7.2.

On 28 January 2013 17:18, Matt Turner <mattst88@gmail.com> wrote:

> On Mon, Jan 28, 2013 at 5:11 PM, Tom Gall <tom.gall@linaro.org> wrote:
> > Sure, but tell that to the compiler.
> >
> > It is bad form to not have a return at the end of a function which is
> > supposed to return something.  The other way perhaps is just knock out
> > the default case and return that. Either way.
>
> % cat t.cpp
> #include <assert.h>
>
> int f(int n) {
>         switch (n) {
>         case 1:
>                 return 1;
>         default:
>                 assert(!"FAIL");
>         }
> }
>
> % gcc -O2 -Wall -pedantic -Wreturn-type -Wno-return-type -c t.cpp
>
> <no output>
>
> What flags do I have to use to get an error or warning? I tried with
> an without optimization, C and C++, and gcc versions 4.5.4, 4.6.3, and
> 4.7.2.
>

Keep in mind that when the "NDEBUG" symbol is defined, assertions have no
effect, so it's possible for control flow to get past an assert(!"FAIL")
statement.

This produces the warning for me:

gcc -DNDEBUG -Wreturn-type -c t.cpp


Given that some people compile Piglit with NDEBUG and some people compile
it without, I think Tom's patches 2/3 and 3/3 are reasonable.