All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t v5] libs/igt_core.c: Fix compile warnings in igt_core.c
@ 2015-06-12 10:37 Derek Morton
  2015-06-15 14:39 ` Daniel Vetter
  0 siblings, 1 reply; 4+ messages in thread
From: Derek Morton @ 2015-06-12 10:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: thomas.wood

Fixed variables incorrectly declared as int instead of size_t.

v2: Addressed comments from Tim Gore
v3: Removed 'unused parameter' changes
v4: Changed to size_t
v5: Moved declarations out of for loops

Signed-off-by: Derek Morton <derek.j.morton@intel.com>
---
 lib/igt_core.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 8a1a249..eb0cb21 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1104,7 +1104,9 @@ static pid_t helper_process_pids[] =
 
 static void reset_helper_process_list(void)
 {
-	for (int i = 0; i < ARRAY_SIZE(helper_process_pids); i++)
+	size_t i;
+
+	for (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)
 {
+	size_t i;
+
 	/* Inside a signal handler, play safe */
-	for (int i = 0; i < ARRAY_SIZE(helper_process_pids); i++) {
+	for (i = 0; i < ARRAY_SIZE(helper_process_pids); i++) {
 		pid_t pid = helper_process_pids[i];
 		if (pid != -1) {
 			kill(pid, SIGTERM);
@@ -1376,10 +1380,10 @@ static void restore_sig_handler(int sig_num)
 
 static void restore_all_sig_handler(void)
 {
-	int i;
+	size_t 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;
+	size_t 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;
+	size_t 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;
+	size_t i;
 
 	if (exit_handler_disabled)
 		return;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH i-g-t v5] libs/igt_core.c: Fix compile warnings in igt_core.c
  2015-06-12 10:37 [PATCH i-g-t v5] libs/igt_core.c: Fix compile warnings in igt_core.c Derek Morton
@ 2015-06-15 14:39 ` Daniel Vetter
  2015-06-23 15:15   ` Morton, Derek J
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2015-06-15 14:39 UTC (permalink / raw)
  To: Derek Morton; +Cc: intel-gfx, thomas.wood

On Fri, Jun 12, 2015 at 11:37:37AM +0100, Derek Morton wrote:
> Fixed variables incorrectly declared as int instead of size_t.
> 
> v2: Addressed comments from Tim Gore
> v3: Removed 'unused parameter' changes
> v4: Changed to size_t
> v5: Moved declarations out of for loops
> 
> Signed-off-by: Derek Morton <derek.j.morton@intel.com>
> ---
>  lib/igt_core.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 8a1a249..eb0cb21 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -1104,7 +1104,9 @@ static pid_t helper_process_pids[] =
>  
>  static void reset_helper_process_list(void)
>  {
> -	for (int i = 0; i < ARRAY_SIZE(helper_process_pids); i++)
> +	size_t i;
> +
> +	for (i = 0; i < ARRAY_SIZE(helper_process_pids); i++)

I still maintain that your gcc is silly since it's trivial to prove that i
will fit into an int.

>  		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)
>  {
> +	size_t i;
> +
>  	/* Inside a signal handler, play safe */
> -	for (int i = 0; i < ARRAY_SIZE(helper_process_pids); i++) {
> +	for (i = 0; i < ARRAY_SIZE(helper_process_pids); i++) {
>  		pid_t pid = helper_process_pids[i];
>  		if (pid != -1) {
>  			kill(pid, SIGTERM);
> @@ -1376,10 +1380,10 @@ static void restore_sig_handler(int sig_num)
>  
>  static void restore_all_sig_handler(void)
>  {
> -	int i;
> +	size_t i;
>  
>  	for (i = 0; i < ARRAY_SIZE(orig_sig); i++)
> -		restore_sig_handler(i);
> +		restore_sig_handler((int)i);

Even more so if you have to cast back to the real value like here.
-Daniel

>  }
>  
>  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;
> +	size_t 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;
> +	size_t 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;
> +	size_t i;
>  
>  	if (exit_handler_disabled)
>  		return;
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH i-g-t v5] libs/igt_core.c: Fix compile warnings in igt_core.c
  2015-06-15 14:39 ` Daniel Vetter
@ 2015-06-23 15:15   ` Morton, Derek J
  2015-06-23 15:42     ` Chris Wilson
  0 siblings, 1 reply; 4+ messages in thread
From: Morton, Derek J @ 2015-06-23 15:15 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, Wood, Thomas

>
>
>-----Original Message-----
>From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
>Sent: Monday, June 15, 2015 3:39 PM
>To: Morton, Derek J
>Cc: intel-gfx@lists.freedesktop.org; Wood, Thomas
>Subject: Re: [Intel-gfx] [PATCH i-g-t v5] libs/igt_core.c: Fix compile warnings in igt_core.c
>
>On Fri, Jun 12, 2015 at 11:37:37AM +0100, Derek Morton wrote:
>> Fixed variables incorrectly declared as int instead of size_t.
>> 
>> v2: Addressed comments from Tim Gore
>> v3: Removed 'unused parameter' changes
>> v4: Changed to size_t
>> v5: Moved declarations out of for loops
>> 
>> Signed-off-by: Derek Morton <derek.j.morton@intel.com>
>> ---
>>  lib/igt_core.c | 18 +++++++++++-------
>>  1 file changed, 11 insertions(+), 7 deletions(-)
>> 
>> diff --git a/lib/igt_core.c b/lib/igt_core.c index 8a1a249..eb0cb21 
>> 100644
>> --- a/lib/igt_core.c
>> +++ b/lib/igt_core.c
>> @@ -1104,7 +1104,9 @@ static pid_t helper_process_pids[] =
>>  
>>  static void reset_helper_process_list(void)  {
>> -	for (int i = 0; i < ARRAY_SIZE(helper_process_pids); i++)
>> +	size_t i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(helper_process_pids); i++)
>
>I still maintain that your gcc is silly since it's trivial to prove that i will fit into an int.

I don't think your compiler makes any checks to see if it will fit. It would just silently truncate the data if it did not. This is about good coding practice in general. Refusing to fix false positives hides real warnings in the sea of compile routput.
>
>>  		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)  {
>> +	size_t i;
>> +
>>  	/* Inside a signal handler, play safe */
>> -	for (int i = 0; i < ARRAY_SIZE(helper_process_pids); i++) {
>> +	for (i = 0; i < ARRAY_SIZE(helper_process_pids); i++) {
>>  		pid_t pid = helper_process_pids[i];
>>  		if (pid != -1) {
>>  			kill(pid, SIGTERM);
>> @@ -1376,10 +1380,10 @@ static void restore_sig_handler(int sig_num)
>>  
>>  static void restore_all_sig_handler(void)  {
>> -	int i;
>> +	size_t i;
>>  
>>  	for (i = 0; i < ARRAY_SIZE(orig_sig); i++)
>> -		restore_sig_handler(i);
>> +		restore_sig_handler((int)i);
>
>Even more so if you have to cast back to the real value like here.

Would you prefer I cast the ARRAY_SIZE to an int instead?

//Derek

>-Daniel
>
>>  }
>>  
>>  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;
>> +	size_t 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;
>> +	size_t 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;
>> +	size_t i;
>>  
>>  	if (exit_handler_disabled)
>>  		return;
>> --
>> 1.9.1
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>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

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH i-g-t v5] libs/igt_core.c: Fix compile warnings in igt_core.c
  2015-06-23 15:15   ` Morton, Derek J
@ 2015-06-23 15:42     ` Chris Wilson
  0 siblings, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2015-06-23 15:42 UTC (permalink / raw)
  To: Morton, Derek J; +Cc: intel-gfx, Wood, Thomas

On Tue, Jun 23, 2015 at 03:15:40PM +0000, Morton, Derek J wrote:
> >
> >
> >-----Original Message-----
> >From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> >Sent: Monday, June 15, 2015 3:39 PM
> >To: Morton, Derek J
> >Cc: intel-gfx@lists.freedesktop.org; Wood, Thomas
> >Subject: Re: [Intel-gfx] [PATCH i-g-t v5] libs/igt_core.c: Fix compile warnings in igt_core.c
> >
> >On Fri, Jun 12, 2015 at 11:37:37AM +0100, Derek Morton wrote:
> >> Fixed variables incorrectly declared as int instead of size_t.
> >> 
> >> v2: Addressed comments from Tim Gore
> >> v3: Removed 'unused parameter' changes
> >> v4: Changed to size_t
> >> v5: Moved declarations out of for loops
> >> 
> >> Signed-off-by: Derek Morton <derek.j.morton@intel.com>
> >> ---
> >>  lib/igt_core.c | 18 +++++++++++-------
> >>  1 file changed, 11 insertions(+), 7 deletions(-)
> >> 
> >> diff --git a/lib/igt_core.c b/lib/igt_core.c index 8a1a249..eb0cb21 
> >> 100644
> >> --- a/lib/igt_core.c
> >> +++ b/lib/igt_core.c
> >> @@ -1104,7 +1104,9 @@ static pid_t helper_process_pids[] =
> >>  
> >>  static void reset_helper_process_list(void)  {
> >> -	for (int i = 0; i < ARRAY_SIZE(helper_process_pids); i++)
> >> +	size_t i;
> >> +
> >> +	for (i = 0; i < ARRAY_SIZE(helper_process_pids); i++)
> >
> >I still maintain that your gcc is silly since it's trivial to prove that i will fit into an int.
> 
> I don't think your compiler makes any checks to see if it will fit. It would just silently truncate the data if it did not. This is about good coding practice in general. Refusing to fix false positives hides real warnings in the sea of compile routput.

test.c:5:16: warning: comparison is always true due to limited range of
data type [-Wtype-limits]
  for (int i = 0; i < (unsigned)-1; i++)

If gcc can do it, why can't your static analyser?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-06-23 15:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-12 10:37 [PATCH i-g-t v5] libs/igt_core.c: Fix compile warnings in igt_core.c Derek Morton
2015-06-15 14:39 ` Daniel Vetter
2015-06-23 15:15   ` Morton, Derek J
2015-06-23 15:42     ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.