[Intel-gfx] [PATCH i-g-t v5] libs/igt_core.c: Fix compile warnings in igt_core.c

[Morton, Derek J]

>
>
>-----Original Message-----
>From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel Vetter
>Sent: Monday, June 15, 2015 3:39 PM
>To: Morton, Derek J
>Cc: intel-gfx at 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 at 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Daniel Vetter
>Software Engineer, Intel Corporation
>http://blog.ffwll.ch
>