Re: [Intel-gfx] [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c
On Tue, May 19, 2015 at 01:35:41PM +, Gore, Tim wrote:
-Original Message-
From: Morton, Derek J
Sent: Tuesday, May 19, 2015 12:21 PM
To: intel-gfx@lists.freedesktop.org
Cc: Wood, Thomas; Gore, Tim; Morton, Derek J
Subject: [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c
Fixed variables incorrectly declared as signed instead of unsigned.
Which kind of compiler warning is this? Imo
for (unsigned int i = 0; i something; i++)
is just not C style, the int there is perfectly fine. We've had this
problem before with Android going ridiculously overboard with compiler
warnings, and the solution back then was to remove all the silly ones for
igt. It smells like the same is appropriate for this one here too.
Fixed 'unused parameter' warning from signal handlers that were not using
the signal parameter.
Yeah unused variable because you compile out assert isn't good, it will at
least break a bunch of library unit tests (the ones in lib/tests). I guess
you don't run them in your Android builds?
-Daniel
Signed-off-by: Derek Morton derek.j.mor...@intel.com
---
lib/igt_core.c | 24 +---
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c index 8a1a249..fb0b634 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -440,6 +440,8 @@ static void low_mem_killer_disable(bool disable)
bool igt_exit_called; static void common_exit_handler(int sig) {
+ (void)sig; /* Not used, suppress warning */
+
if (!igt_only_list_subtests()) {
low_mem_killer_disable(false);
}
@@ -1104,7 +1106,7 @@ static pid_t helper_process_pids[] =
static void reset_helper_process_list(void) {
- for (int i = 0; i ARRAY_SIZE(helper_process_pids); i++)
+ for (unsigned int i = 0; i ARRAY_SIZE(helper_process_pids); i++)
helper_process_pids[i] = -1;
helper_process_count = 0;
}
@@ -1121,8 +1123,10 @@ static int __waitpid(pid_t pid)
static void fork_helper_exit_handler(int sig) {
+ (void)sig; /* Not used, suppress warning */
+
/* Inside a signal handler, play safe */
- for (int i = 0; i ARRAY_SIZE(helper_process_pids); i++) {
+ for (unsigned int i = 0; i ARRAY_SIZE(helper_process_pids); i++) {
pid_t pid = helper_process_pids[i];
if (pid != -1) {
kill(pid, SIGTERM);
@@ -1227,6 +1231,8 @@ static void children_exit_handler(int sig) {
int status;
+ (void)sig; /* Not used, suppress warning */
+
/* The exit handler can be called from a fatal signal, so play safe */
while (num_test_children-- wait(status))
;
@@ -1376,16 +1382,18 @@ static void restore_sig_handler(int sig_num)
static void restore_all_sig_handler(void) {
- int i;
+ unsigned int i;
for (i = 0; i ARRAY_SIZE(orig_sig); i++)
- restore_sig_handler(i);
+ restore_sig_handler((int)i);
}
static void call_exit_handlers(int sig) {
int i;
+ (void)sig; /* Not used, suppress warning */
+
if (!exit_handler_count) {
return;
}
@@ -1419,7 +1427,7 @@ static bool crash_signal(int sig)
static void fatal_sig_handler(int sig)
{
- int i;
+ unsigned int i;
for (i = 0; i ARRAY_SIZE(handled_signals); i++) {
if (handled_signals[i].number != sig) @@ -1481,7 +1489,7
@@ static void fatal_sig_handler(int sig)
*/
void igt_install_exit_handler(igt_exit_handler_t fn) {
- int i;
+ unsigned int i;
for (i = 0; i exit_handler_count; i++)
if (exit_handler_fn[i] == fn)
@@ -1521,7 +1529,7 @@ err:
void igt_disable_exit_handler(void)
{
sigset_t set;
- int i;
+ unsigned int i;
if (exit_handler_disabled)
return;
@@ -1724,6 +1732,8 @@ out:
static void igt_alarm_handler(int signal) {
+ (void)signal; /* Not used, suppress warning */
+
igt_info(Timed out\n);
/* exit with failure status */
--
1.9.1
In two of the functions where you use (void)sig, sig is in fact used.
In common_exit_handler it is used in the assert at the end. If this creates
A warning (which I observe also) it indicates that asserts are off which we
Probably don't want. The build explicitly uses -include check-ndebug.h
To create a compile error if NDEBUG is set, but this does not seem to be
Working, maybe due to the Android.mk also specifying -UNDEBUG.
Sorting this is probably for a separate patch.but I think you should remove
The (void)sig from common_exit_handler, so the resulting warning will
Remind us to fix the assert issue.
Also, in call_exit_handlers the sig parameter is used, so the (void)sig is
Not needed.
Tim
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
Re: [Intel-gfx] [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c
On Tue, 19 May 2015, Derek Morton derek.j.mor...@intel.com wrote:
Fixed variables incorrectly declared as signed instead of unsigned.
Fixed 'unused parameter' warning from signal handlers that were
not using the signal parameter.
v2: Addressed comments from Tim Gore
Signed-off-by: Derek Morton derek.j.mor...@intel.com
---
lib/igt_core.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 8a1a249..62b1e6a 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1104,7 +1104,7 @@ static pid_t helper_process_pids[] =
static void reset_helper_process_list(void)
{
- for (int i = 0; i ARRAY_SIZE(helper_process_pids); i++)
+ for (unsigned int i = 0; i ARRAY_SIZE(helper_process_pids); i++)
helper_process_pids[i] = -1;
helper_process_count = 0;
}
@@ -1121,8 +1121,10 @@ static int __waitpid(pid_t pid)
static void fork_helper_exit_handler(int sig)
{
+ (void)sig; /* Not used, suppress warning */
No, this is not the way to go.
If you really want to get rid of the warning (not enabled by default),
you should define a macro:
#define unused(x) x __attribute__ ((unused))
and make the change:
-static void fork_helper_exit_handler(int sig)
+static void fork_helper_exit_handler(unused(int sig))
And to make that right, you should define unused depending on the
compiler you're using.
+
/* Inside a signal handler, play safe */
- for (int i = 0; i ARRAY_SIZE(helper_process_pids); i++) {
+ for (unsigned int i = 0; i ARRAY_SIZE(helper_process_pids); i++) {
pid_t pid = helper_process_pids[i];
if (pid != -1) {
kill(pid, SIGTERM);
@@ -1227,6 +1229,8 @@ static void children_exit_handler(int sig)
{
int status;
+ (void)sig; /* Not used, suppress warning */
+
/* The exit handler can be called from a fatal signal, so play safe */
while (num_test_children-- wait(status))
;
@@ -1376,10 +1380,10 @@ static void restore_sig_handler(int sig_num)
static void restore_all_sig_handler(void)
{
- int i;
+ unsigned int i;
for (i = 0; i ARRAY_SIZE(orig_sig); i++)
- restore_sig_handler(i);
+ restore_sig_handler((int)i);
I think the warning is much better than having an explicit cast in the
code like this.
BR,
Jani.
}
static void call_exit_handlers(int sig)
@@ -1419,7 +1423,7 @@ static bool crash_signal(int sig)
static void fatal_sig_handler(int sig)
{
- int i;
+ unsigned int i;
for (i = 0; i ARRAY_SIZE(handled_signals); i++) {
if (handled_signals[i].number != sig)
@@ -1481,7 +1485,7 @@ static void fatal_sig_handler(int sig)
*/
void igt_install_exit_handler(igt_exit_handler_t fn)
{
- int i;
+ unsigned int i;
for (i = 0; i exit_handler_count; i++)
if (exit_handler_fn[i] == fn)
@@ -1521,7 +1525,7 @@ err:
void igt_disable_exit_handler(void)
{
sigset_t set;
- int i;
+ unsigned int i;
if (exit_handler_disabled)
return;
@@ -1724,6 +1728,8 @@ out:
static void igt_alarm_handler(int signal)
{
+ (void)signal; /* Not used, suppress warning */
+
igt_info(Timed out\n);
/* exit with failure status */
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c
-Original Message-
From: Lespiau, Damien
Sent: Wednesday, May 20, 2015 10:48 AM
To: Morton, Derek J
Cc: Daniel Vetter; Gore, Tim; intel-gfx@lists.freedesktop.org; Wood, Thomas
Subject: Re: [Intel-gfx] [PATCH i-g-t] libs/igt_core.c: Fix compile warnings
in
igt_core.c
On Wed, May 20, 2015 at 08:37:56AM +, Morton, Derek J wrote:
-Original Message-
From: Morton, Derek J
Sent: Tuesday, May 19, 2015 12:21 PM
To: intel-gfx@lists.freedesktop.org
Cc: Wood, Thomas; Gore, Tim; Morton, Derek J
Subject: [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in
igt_core.c
Fixed variables incorrectly declared as signed instead of unsigned.
Which kind of compiler warning is this? Imo
for (unsigned int i = 0; i something; i++)
is just not C style, the int there is perfectly fine. We've had this
problem before with Android going ridiculously overboard with
compiler warnings, and the solution back then was to remove all the
silly ones for igt. It smells like the same is appropriate for this
one here too.
The warning occurs because 'something' is of an unsigned type. In this
case the macro ARRAY_SIZE uses sizeof() which returns an unsigned
type. Implicit conversion between signed and unsigned types has always
resulted in compiler warnings in my experience. It is not something
android specific.
unsigned int like that is C99 and the sizeof operator is a size_t, so maybe
use
that instead of unsigned int?
Fixed 'unused parameter' warning from signal handlers that were
not using the signal parameter.
Yeah unused variable because you compile out assert isn't good, it
will at least break a bunch of library unit tests (the ones in
lib/tests). I guess you don't run them in your Android builds?
-Daniel
I have no idea why the asserts are compiled out for android. Tim had
some suggestions which need investigating. A subject for another patch
I guess. We do not run the unit tests on android because it has not
been possible to run them since they were moved to libs/test. The
android make file was never updated when they were moved. I need to
look at look at writing a new unit test for the fatal_signal_handler
so will look at getting them to build for android at the same time.
As the unused parameter changes are more controversial I will remove
them for now and update the patch to just fix the signed / unsigned
warnings.
FWIW, I'd used the gcc unused attribute for things like that instead of those
void expressions.
#define __unused __attribute__((unused))
void foo(int bar __unused)
{
...
}
--
Damien
My penny's worth is this. Most of the time assigning between signed and unsigned
is just due to sloppy coding. We all do it for sure, and I see it everywhere.
But it can
lead to problems and it is very kind of the compiler to warn you. By default
GCC only
warns you in C++, but I think Google must have tweaked their GCC build, I cannot
see it turned on in our build command. Fixing these is trivial, does not
introduce
extra code and reduces the noise when building for Android. And its even good
style (unlike this sentence). I agree with Damien that size_t is probably better
in this case.
Tim
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c
On Wed, 20 May 2015, Gore, Tim tim.g...@intel.com wrote:
-Original Message-
From: Lespiau, Damien
Sent: Wednesday, May 20, 2015 10:48 AM
To: Morton, Derek J
Cc: Daniel Vetter; Gore, Tim; intel-gfx@lists.freedesktop.org; Wood, Thomas
Subject: Re: [Intel-gfx] [PATCH i-g-t] libs/igt_core.c: Fix compile warnings
in
igt_core.c
On Wed, May 20, 2015 at 08:37:56AM +, Morton, Derek J wrote:
-Original Message-
From: Morton, Derek J
Sent: Tuesday, May 19, 2015 12:21 PM
To: intel-gfx@lists.freedesktop.org
Cc: Wood, Thomas; Gore, Tim; Morton, Derek J
Subject: [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in
igt_core.c
Fixed variables incorrectly declared as signed instead of unsigned.
Which kind of compiler warning is this? Imo
for (unsigned int i = 0; i something; i++)
is just not C style, the int there is perfectly fine. We've had this
problem before with Android going ridiculously overboard with
compiler warnings, and the solution back then was to remove all the
silly ones for igt. It smells like the same is appropriate for this
one here too.
The warning occurs because 'something' is of an unsigned type. In this
case the macro ARRAY_SIZE uses sizeof() which returns an unsigned
type. Implicit conversion between signed and unsigned types has always
resulted in compiler warnings in my experience. It is not something
android specific.
unsigned int like that is C99 and the sizeof operator is a size_t, so maybe
use
that instead of unsigned int?
Fixed 'unused parameter' warning from signal handlers that were
not using the signal parameter.
Yeah unused variable because you compile out assert isn't good, it
will at least break a bunch of library unit tests (the ones in
lib/tests). I guess you don't run them in your Android builds?
-Daniel
I have no idea why the asserts are compiled out for android. Tim had
some suggestions which need investigating. A subject for another patch
I guess. We do not run the unit tests on android because it has not
been possible to run them since they were moved to libs/test. The
android make file was never updated when they were moved. I need to
look at look at writing a new unit test for the fatal_signal_handler
so will look at getting them to build for android at the same time.
As the unused parameter changes are more controversial I will remove
them for now and update the patch to just fix the signed / unsigned
warnings.
FWIW, I'd used the gcc unused attribute for things like that instead of those
void expressions.
#define __unused __attribute__((unused))
void foo(int bar __unused)
{
...
}
--
Damien
My penny's worth is this. Most of the time assigning between signed and
unsigned
is just due to sloppy coding. We all do it for sure, and I see it everywhere.
But it can
lead to problems and it is very kind of the compiler to warn you. By default
GCC only
warns you in C++, but I think Google must have tweaked their GCC build, I
cannot
see it turned on in our build command. Fixing these is trivial, does not
introduce
extra code and reduces the noise when building for Android. And its even good
style (unlike this sentence). I agree with Damien that size_t is probably
better
in this case.
Like both Chris and I said in reply to different versions of the patch,
having to use an explicit cast because of the signed-unsigned change is
bad too. Worse, I think.
BR,
Jani.
Tim
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c
On Wed, May 20, 2015 at 08:37:56AM +, Morton, Derek J wrote: -Original Message- From: Morton, Derek J Sent: Tuesday, May 19, 2015 12:21 PM To: intel-gfx@lists.freedesktop.org Cc: Wood, Thomas; Gore, Tim; Morton, Derek J Subject: [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c Fixed variables incorrectly declared as signed instead of unsigned. Which kind of compiler warning is this? Imo for (unsigned int i = 0; i something; i++) is just not C style, the int there is perfectly fine. We've had this problem before with Android going ridiculously overboard with compiler warnings, and the solution back then was to remove all the silly ones for igt. It smells like the same is appropriate for this one here too. The warning occurs because 'something' is of an unsigned type. In this case the macro ARRAY_SIZE uses sizeof() which returns an unsigned type. Implicit conversion between signed and unsigned types has always resulted in compiler warnings in my experience. It is not something android specific. I don't have them here when building igt, and ARRAY_SIZE is also extensively used in the kernel, also with size_of. This really sounds like Android again going overboard with enabling compiler warnings to me. Also the warning is silly here, since ARRAY_SIZE is statically known and gcc can figure out that there's no overflow possible. The warning level in kernel is such that you get overflow warnings only when gcc can prove that there's an overflow. Which makes sense, but this here imo really doesn't. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c
On Wed, May 20, 2015 at 08:37:56AM +, Morton, Derek J wrote:
-Original Message-
From: Morton, Derek J
Sent: Tuesday, May 19, 2015 12:21 PM
To: intel-gfx@lists.freedesktop.org
Cc: Wood, Thomas; Gore, Tim; Morton, Derek J
Subject: [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in
igt_core.c
Fixed variables incorrectly declared as signed instead of unsigned.
Which kind of compiler warning is this? Imo
for (unsigned int i = 0; i something; i++)
is just not C style, the int there is perfectly fine. We've had this
problem before with Android going ridiculously overboard with
compiler warnings, and the solution back then was to remove all the
silly ones for igt. It smells like the same is appropriate for this
one here too.
The warning occurs because 'something' is of an unsigned type. In this
case the macro ARRAY_SIZE uses sizeof() which returns an unsigned
type. Implicit conversion between signed and unsigned types has always
resulted in compiler warnings in my experience. It is not something
android specific.
unsigned int like that is C99 and the sizeof operator is a size_t, so
maybe use that instead of unsigned int?
Fixed 'unused parameter' warning from signal handlers that were
not using the signal parameter.
Yeah unused variable because you compile out assert isn't good, it
will at least break a bunch of library unit tests (the ones in
lib/tests). I guess you don't run them in your Android builds?
-Daniel
I have no idea why the asserts are compiled out for android. Tim had
some suggestions which need investigating. A subject for another patch
I guess. We do not run the unit tests on android because it has not
been possible to run them since they were moved to libs/test. The
android make file was never updated when they were moved. I need to
look at look at writing a new unit test for the fatal_signal_handler
so will look at getting them to build for android at the same time.
As the unused parameter changes are more controversial I will remove
them for now and update the patch to just fix the signed / unsigned
warnings.
FWIW, I'd used the gcc unused attribute for things like that instead of
those void expressions.
#define __unused __attribute__((unused))
void foo(int bar __unused)
{
...
}
--
Damien
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c
-Original Message-
From: Morton, Derek J
Sent: Tuesday, May 19, 2015 12:21 PM
To: intel-gfx@lists.freedesktop.org
Cc: Wood, Thomas; Gore, Tim; Morton, Derek J
Subject: [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in
igt_core.c
Fixed variables incorrectly declared as signed instead of unsigned.
Which kind of compiler warning is this? Imo
for (unsigned int i = 0; i something; i++)
is just not C style, the int there is perfectly fine. We've had this problem
before with Android going ridiculously overboard with compiler warnings, and
the solution back then was to remove all the silly ones for igt. It smells
like the same is appropriate for this one here too.
The warning occurs because 'something' is of an unsigned type. In this case the
macro ARRAY_SIZE uses sizeof() which returns an unsigned type. Implicit
conversion between signed and unsigned types has always resulted in compiler
warnings in my experience. It is not something android specific.
Fixed 'unused parameter' warning from signal handlers that were not
using the signal parameter.
Yeah unused variable because you compile out assert isn't good, it will at
least break a bunch of library unit tests (the ones in lib/tests). I guess
you don't run them in your Android builds?
-Daniel
I have no idea why the asserts are compiled out for android. Tim had some
suggestions which need investigating. A subject for another patch I guess.
We do not run the unit tests on android because it has not been possible to run
them since they were moved to libs/test. The android make file was never
updated when they were moved.
I need to look at look at writing a new unit test for the fatal_signal_handler
so will look at getting them to build for android at the same time.
As the unused parameter changes are more controversial I will remove them for
now and update the patch to just fix the signed / unsigned warnings.
Signed-off-by: Derek Morton derek.j.mor...@intel.com
---
lib/igt_core.c | 24 +---
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c index 8a1a249..fb0b634
100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -440,6 +440,8 @@ static void low_mem_killer_disable(bool disable)
bool igt_exit_called; static void common_exit_handler(int sig) {
+ (void)sig; /* Not used, suppress warning */
+
if (!igt_only_list_subtests()) {
low_mem_killer_disable(false);
}
@@ -1104,7 +1106,7 @@ static pid_t helper_process_pids[] =
static void reset_helper_process_list(void) {
- for (int i = 0; i ARRAY_SIZE(helper_process_pids); i++)
+ for (unsigned int i = 0; i ARRAY_SIZE(helper_process_pids); i++)
helper_process_pids[i] = -1;
helper_process_count = 0;
}
@@ -1121,8 +1123,10 @@ static int __waitpid(pid_t pid)
static void fork_helper_exit_handler(int sig) {
+ (void)sig; /* Not used, suppress warning */
+
/* Inside a signal handler, play safe */
- for (int i = 0; i ARRAY_SIZE(helper_process_pids); i++) {
+ for (unsigned int i = 0; i ARRAY_SIZE(helper_process_pids); i++)
+{
pid_t pid = helper_process_pids[i];
if (pid != -1) {
kill(pid, SIGTERM);
@@ -1227,6 +1231,8 @@ static void children_exit_handler(int sig) {
int status;
+ (void)sig; /* Not used, suppress warning */
+
/* The exit handler can be called from a fatal signal, so play safe */
while (num_test_children-- wait(status))
;
@@ -1376,16 +1382,18 @@ static void restore_sig_handler(int sig_num)
static void restore_all_sig_handler(void) {
- int i;
+ unsigned int i;
for (i = 0; i ARRAY_SIZE(orig_sig); i++)
- restore_sig_handler(i);
+ restore_sig_handler((int)i);
}
static void call_exit_handlers(int sig) {
int i;
+ (void)sig; /* Not used, suppress warning */
+
if (!exit_handler_count) {
return;
}
@@ -1419,7 +1427,7 @@ static bool crash_signal(int sig)
static void fatal_sig_handler(int sig) {
- int i;
+ unsigned int i;
for (i = 0; i ARRAY_SIZE(handled_signals); i++) {
if (handled_signals[i].number != sig) @@ -1481,7 +1489,7 @@
static void fatal_sig_handler(int sig)
*/
void igt_install_exit_handler(igt_exit_handler_t fn) {
- int i;
+ unsigned int i;
for (i = 0; i exit_handler_count; i++)
if (exit_handler_fn[i] == fn)
@@ -1521,7 +1529,7 @@ err:
void igt_disable_exit_handler(void) {
sigset_t set;
- int i;
+ unsigned int i;
if (exit_handler_disabled)
return;
@@ -1724,6 +1732,8 @@ out:
static void igt_alarm_handler(int signal) {
+ (void)signal; /* Not used, suppress warning */
+
igt_info(Timed out\n);
/* exit with failure status */
--
1.9.1
In two of the functions
[Intel-gfx] [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c
Fixed variables incorrectly declared as signed instead of unsigned.
Fixed 'unused parameter' warning from signal handlers that were
not using the signal parameter.
Signed-off-by: Derek Morton derek.j.mor...@intel.com
---
lib/igt_core.c | 24 +---
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 8a1a249..fb0b634 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -440,6 +440,8 @@ static void low_mem_killer_disable(bool disable)
bool igt_exit_called;
static void common_exit_handler(int sig)
{
+ (void)sig; /* Not used, suppress warning */
+
if (!igt_only_list_subtests()) {
low_mem_killer_disable(false);
}
@@ -1104,7 +1106,7 @@ static pid_t helper_process_pids[] =
static void reset_helper_process_list(void)
{
- for (int i = 0; i ARRAY_SIZE(helper_process_pids); i++)
+ for (unsigned int i = 0; i ARRAY_SIZE(helper_process_pids); i++)
helper_process_pids[i] = -1;
helper_process_count = 0;
}
@@ -1121,8 +1123,10 @@ static int __waitpid(pid_t pid)
static void fork_helper_exit_handler(int sig)
{
+ (void)sig; /* Not used, suppress warning */
+
/* Inside a signal handler, play safe */
- for (int i = 0; i ARRAY_SIZE(helper_process_pids); i++) {
+ for (unsigned int i = 0; i ARRAY_SIZE(helper_process_pids); i++) {
pid_t pid = helper_process_pids[i];
if (pid != -1) {
kill(pid, SIGTERM);
@@ -1227,6 +1231,8 @@ static void children_exit_handler(int sig)
{
int status;
+ (void)sig; /* Not used, suppress warning */
+
/* The exit handler can be called from a fatal signal, so play safe */
while (num_test_children-- wait(status))
;
@@ -1376,16 +1382,18 @@ static void restore_sig_handler(int sig_num)
static void restore_all_sig_handler(void)
{
- int i;
+ unsigned int i;
for (i = 0; i ARRAY_SIZE(orig_sig); i++)
- restore_sig_handler(i);
+ restore_sig_handler((int)i);
}
static void call_exit_handlers(int sig)
{
int i;
+ (void)sig; /* Not used, suppress warning */
+
if (!exit_handler_count) {
return;
}
@@ -1419,7 +1427,7 @@ static bool crash_signal(int sig)
static void fatal_sig_handler(int sig)
{
- int i;
+ unsigned int i;
for (i = 0; i ARRAY_SIZE(handled_signals); i++) {
if (handled_signals[i].number != sig)
@@ -1481,7 +1489,7 @@ static void fatal_sig_handler(int sig)
*/
void igt_install_exit_handler(igt_exit_handler_t fn)
{
- int i;
+ unsigned int i;
for (i = 0; i exit_handler_count; i++)
if (exit_handler_fn[i] == fn)
@@ -1521,7 +1529,7 @@ err:
void igt_disable_exit_handler(void)
{
sigset_t set;
- int i;
+ unsigned int i;
if (exit_handler_disabled)
return;
@@ -1724,6 +1732,8 @@ out:
static void igt_alarm_handler(int signal)
{
+ (void)signal; /* Not used, suppress warning */
+
igt_info(Timed out\n);
/* exit with failure status */
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c
-Original Message-
From: Morton, Derek J
Sent: Tuesday, May 19, 2015 12:21 PM
To: intel-gfx@lists.freedesktop.org
Cc: Wood, Thomas; Gore, Tim; Morton, Derek J
Subject: [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c
Fixed variables incorrectly declared as signed instead of unsigned.
Fixed 'unused parameter' warning from signal handlers that were not using
the signal parameter.
Signed-off-by: Derek Morton derek.j.mor...@intel.com
---
lib/igt_core.c | 24 +---
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c index 8a1a249..fb0b634 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -440,6 +440,8 @@ static void low_mem_killer_disable(bool disable)
bool igt_exit_called; static void common_exit_handler(int sig) {
+ (void)sig; /* Not used, suppress warning */
+
if (!igt_only_list_subtests()) {
low_mem_killer_disable(false);
}
@@ -1104,7 +1106,7 @@ static pid_t helper_process_pids[] =
static void reset_helper_process_list(void) {
- for (int i = 0; i ARRAY_SIZE(helper_process_pids); i++)
+ for (unsigned int i = 0; i ARRAY_SIZE(helper_process_pids); i++)
helper_process_pids[i] = -1;
helper_process_count = 0;
}
@@ -1121,8 +1123,10 @@ static int __waitpid(pid_t pid)
static void fork_helper_exit_handler(int sig) {
+ (void)sig; /* Not used, suppress warning */
+
/* Inside a signal handler, play safe */
- for (int i = 0; i ARRAY_SIZE(helper_process_pids); i++) {
+ for (unsigned int i = 0; i ARRAY_SIZE(helper_process_pids); i++) {
pid_t pid = helper_process_pids[i];
if (pid != -1) {
kill(pid, SIGTERM);
@@ -1227,6 +1231,8 @@ static void children_exit_handler(int sig) {
int status;
+ (void)sig; /* Not used, suppress warning */
+
/* The exit handler can be called from a fatal signal, so play safe */
while (num_test_children-- wait(status))
;
@@ -1376,16 +1382,18 @@ static void restore_sig_handler(int sig_num)
static void restore_all_sig_handler(void) {
- int i;
+ unsigned int i;
for (i = 0; i ARRAY_SIZE(orig_sig); i++)
- restore_sig_handler(i);
+ restore_sig_handler((int)i);
}
static void call_exit_handlers(int sig) {
int i;
+ (void)sig; /* Not used, suppress warning */
+
if (!exit_handler_count) {
return;
}
@@ -1419,7 +1427,7 @@ static bool crash_signal(int sig)
static void fatal_sig_handler(int sig)
{
- int i;
+ unsigned int i;
for (i = 0; i ARRAY_SIZE(handled_signals); i++) {
if (handled_signals[i].number != sig) @@ -1481,7 +1489,7
@@ static void fatal_sig_handler(int sig)
*/
void igt_install_exit_handler(igt_exit_handler_t fn) {
- int i;
+ unsigned int i;
for (i = 0; i exit_handler_count; i++)
if (exit_handler_fn[i] == fn)
@@ -1521,7 +1529,7 @@ err:
void igt_disable_exit_handler(void)
{
sigset_t set;
- int i;
+ unsigned int i;
if (exit_handler_disabled)
return;
@@ -1724,6 +1732,8 @@ out:
static void igt_alarm_handler(int signal) {
+ (void)signal; /* Not used, suppress warning */
+
igt_info(Timed out\n);
/* exit with failure status */
--
1.9.1
In two of the functions where you use (void)sig, sig is in fact used.
In common_exit_handler it is used in the assert at the end. If this creates
A warning (which I observe also) it indicates that asserts are off which we
Probably don't want. The build explicitly uses -include check-ndebug.h
To create a compile error if NDEBUG is set, but this does not seem to be
Working, maybe due to the Android.mk also specifying -UNDEBUG.
Sorting this is probably for a separate patch.but I think you should remove
The (void)sig from common_exit_handler, so the resulting warning will
Remind us to fix the assert issue.
Also, in call_exit_handlers the sig parameter is used, so the (void)sig is
Not needed.
Tim
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] libs/igt_core.c: Fix compile warnings in igt_core.c
Fixed variables incorrectly declared as signed instead of unsigned.
Fixed 'unused parameter' warning from signal handlers that were
not using the signal parameter.
v2: Addressed comments from Tim Gore
Signed-off-by: Derek Morton derek.j.mor...@intel.com
---
lib/igt_core.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 8a1a249..62b1e6a 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1104,7 +1104,7 @@ static pid_t helper_process_pids[] =
static void reset_helper_process_list(void)
{
- for (int i = 0; i ARRAY_SIZE(helper_process_pids); i++)
+ for (unsigned int i = 0; i ARRAY_SIZE(helper_process_pids); i++)
helper_process_pids[i] = -1;
helper_process_count = 0;
}
@@ -1121,8 +1121,10 @@ static int __waitpid(pid_t pid)
static void fork_helper_exit_handler(int sig)
{
+ (void)sig; /* Not used, suppress warning */
+
/* Inside a signal handler, play safe */
- for (int i = 0; i ARRAY_SIZE(helper_process_pids); i++) {
+ for (unsigned int i = 0; i ARRAY_SIZE(helper_process_pids); i++) {
pid_t pid = helper_process_pids[i];
if (pid != -1) {
kill(pid, SIGTERM);
@@ -1227,6 +1229,8 @@ static void children_exit_handler(int sig)
{
int status;
+ (void)sig; /* Not used, suppress warning */
+
/* The exit handler can be called from a fatal signal, so play safe */
while (num_test_children-- wait(status))
;
@@ -1376,10 +1380,10 @@ static void restore_sig_handler(int sig_num)
static void restore_all_sig_handler(void)
{
- int i;
+ unsigned int i;
for (i = 0; i ARRAY_SIZE(orig_sig); i++)
- restore_sig_handler(i);
+ restore_sig_handler((int)i);
}
static void call_exit_handlers(int sig)
@@ -1419,7 +1423,7 @@ static bool crash_signal(int sig)
static void fatal_sig_handler(int sig)
{
- int i;
+ unsigned int i;
for (i = 0; i ARRAY_SIZE(handled_signals); i++) {
if (handled_signals[i].number != sig)
@@ -1481,7 +1485,7 @@ static void fatal_sig_handler(int sig)
*/
void igt_install_exit_handler(igt_exit_handler_t fn)
{
- int i;
+ unsigned int i;
for (i = 0; i exit_handler_count; i++)
if (exit_handler_fn[i] == fn)
@@ -1521,7 +1525,7 @@ err:
void igt_disable_exit_handler(void)
{
sigset_t set;
- int i;
+ unsigned int i;
if (exit_handler_disabled)
return;
@@ -1724,6 +1728,8 @@ out:
static void igt_alarm_handler(int signal)
{
+ (void)signal; /* Not used, suppress warning */
+
igt_info(Timed out\n);
/* exit with failure status */
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
