[refpolicy] [PATCH v2] fc_sort: avoid compiler warning/error

[refpolicy] [PATCH v2] fc_sort: avoid compiler warning/error

[Christian Göttsche]

Or maybe just add a comment /*FALLTHRU*/ ?
This should silence the warning.

https://developers.redhat.com/blog/2017/03/10/wimplicit-fallthrough-in-gcc-7/

On 5 Jun 2017 3:04 pm, "Guido Trentalancia via refpolicy" <
refpolicy@oss.tresys.com> wrote:

> Fix a "-Werror=implicit-fallthrough" compiler warning/error on
> the switch statement.
>
> This second version (v2) fixes a bug introduced with the previous
> version.
>
> Signed-off-by: Guido Trentalancia <guido@trentalancia.com>
> ---
>  support/fc_sort.c |    5 +++++
>  1 file changed, 5 insertions(+)
>
> --- a/support/fc_sort.c 2017-03-29 17:57:54.576386420 +0200
> +++ b/support/fc_sort.c 2017-06-05 17:01:44.818724998 +0200
> @@ -292,6 +292,11 @@ void fc_fill_data(file_context_node_t *f
>                         /* If a escape character is found,
>                          *  skip the next character. */
>                         c++;
> +                       /* If no meta character has been found yet,
> +                        * add one to the stem length. */
> +                       if (!fc_node->meta)
> +                               fc_node->stem_len++;
> +                       break;
>                 default:
>                         /* If no meta character has been found yet,
>                          *  add one to the stem length. */
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://oss.tresys.com/pipermail/refpolicy/attachments/20170605/f8516853/attachment-0001.html 

[refpolicy] [PATCH] fc_sort: avoid compiler warning/error

[Guido Trentalancia]

Fix a "-Werror=implicit-fallthrough" compiler warning/error on
the switch statement.

Signed-off-by: Guido Trentalancia <guido@trentalancia.com>
---
 support/fc_sort.c |    1 +
 1 file changed, 1 insertion(+)

--- a/support/fc_sort.c	2017-03-29 17:57:54.576386420 +0200
+++ b/support/fc_sort.c	2017-06-05 16:43:51.623729385 +0200
@@ -292,6 +292,7 @@ void fc_fill_data(file_context_node_t *f
 			/* If a escape character is found,
 			 *  skip the next character. */
 			c++;
+			break;
 		default:
 			/* If no meta character has been found yet,
 			 *  add one to the stem length. */

[refpolicy] [PATCH v2] fc_sort: avoid compiler warning/error

[Guido Trentalancia]

Fix a "-Werror=implicit-fallthrough" compiler warning/error on
the switch statement.

This second version (v2) fixes a bug introduced with the previous
version.

Signed-off-by: Guido Trentalancia <guido@trentalancia.com>
---
 support/fc_sort.c |    5 +++++
 1 file changed, 5 insertions(+)

--- a/support/fc_sort.c	2017-03-29 17:57:54.576386420 +0200
+++ b/support/fc_sort.c	2017-06-05 17:01:44.818724998 +0200
@@ -292,6 +292,11 @@ void fc_fill_data(file_context_node_t *f
 			/* If a escape character is found,
 			 *  skip the next character. */
 			c++;
+			/* If no meta character has been found yet,
+			 * add one to the stem length. */
+			if (!fc_node->meta)
+				fc_node->stem_len++;
+			break;
 		default:
 			/* If no meta character has been found yet,
 			 *  add one to the stem length. */

[refpolicy] [PATCH v2] fc_sort: avoid compiler warning/error

[Guido Trentalancia]

Hello, 

thanks for telling me about this option, but I prefer to leave things as they are, without being too much specific to the particular compiler. 

Regards, 

Guido 

On the 5th of June 2017 16:20:15 CEST, "Christian G?ttsche" <cgzones@googlemail.com> wrote:
>Or maybe just add a comment /*FALLTHRU*/ ?
>This should silence the warning.
>
>https://developers.redhat.com/blog/2017/03/10/wimplicit-fallthrough-in-gcc-7/
>
>On 5 Jun 2017 3:04 pm, "Guido Trentalancia via refpolicy" <
>refpolicy at oss.tresys.com> wrote:
>
>> Fix a "-Werror=implicit-fallthrough" compiler warning/error on
>> the switch statement.
>>
>> This second version (v2) fixes a bug introduced with the previous
>> version.
>>
>> Signed-off-by: Guido Trentalancia <guido@trentalancia.com>
>> ---
>>  support/fc_sort.c |    5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> --- a/support/fc_sort.c 2017-03-29 17:57:54.576386420 +0200
>> +++ b/support/fc_sort.c 2017-06-05 17:01:44.818724998 +0200
>> @@ -292,6 +292,11 @@ void fc_fill_data(file_context_node_t *f
>>                         /* If a escape character is found,
>>                          *  skip the next character. */
>>                         c++;
>> +                       /* If no meta character has been found yet,
>> +                        * add one to the stem length. */
>> +                       if (!fc_node->meta)
>> +                               fc_node->stem_len++;
>> +                       break;
>>                 default:
>>                         /* If no meta character has been found yet,
>>                          *  add one to the stem length. */
>> _______________________________________________
>> refpolicy mailing list
>> refpolicy at oss.tresys.com
>> http://oss.tresys.com/mailman/listinfo/refpolicy
>>

[refpolicy] [PATCH v2] fc_sort: avoid compiler warning/error

[Jason Zaman]

On Mon, Jun 05, 2017 at 05:04:53PM +0200, Guido Trentalancia via refpolicy wrote:
> Fix a "-Werror=implicit-fallthrough" compiler warning/error on
> the switch statement.
> 
> This second version (v2) fixes a bug introduced with the previous
> version.
> 
> Signed-off-by: Guido Trentalancia <guido@trentalancia.com>
> ---
>  support/fc_sort.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> --- a/support/fc_sort.c	2017-03-29 17:57:54.576386420 +0200
> +++ b/support/fc_sort.c	2017-06-05 17:01:44.818724998 +0200
> @@ -292,6 +292,11 @@ void fc_fill_data(file_context_node_t *f
>  			/* If a escape character is found,
>  			 *  skip the next character. */
>  			c++;
> +			/* If no meta character has been found yet,
> +			 * add one to the stem length. */
> +			if (!fc_node->meta)
> +				fc_node->stem_len++;
> +			break;

Why not just add /* -fallthrough */ instead of the break?

Seems more maintainable than duplicating the code in case something
changes later.

https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

-- Jason

>  		default:
>  			/* If no meta character has been found yet,
>  			 *  add one to the stem length. */
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy

[refpolicy] [PATCH v3] fc_sort: avoid compiler warning/error

[Guido Trentalancia]

Fix a "-Werror=implicit-fallthrough" compiler warning/error on
the switch statement.

This third version (v3) fixes a bug introduced in the first
version and improves the style over the second version.

Signed-off-by: Guido Trentalancia <guido@trentalancia.com>
---
 support/fc_sort.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

--- a/support/fc_sort.c	2017-06-05 23:49:57.280624866 +0200
+++ b/support/fc_sort.c	2017-06-06 00:04:48.419621222 +0200
@@ -292,14 +292,16 @@ void fc_fill_data(file_context_node_t *f
 			/* If a escape character is found,
 			 *  skip the next character. */
 			c++;
+			break;
 		default:
-			/* If no meta character has been found yet,
-			 *  add one to the stem length. */
-			if (!fc_node->meta)
-				fc_node->stem_len++;
 			break;
 		}
 
+		/* If no meta character has been found yet,
+		 * add one to the stem length. */
+		if (!fc_node->meta)
+			fc_node->stem_len++;
+
 		fc_node->str_len++;
 		c++;
 	}

[refpolicy] [PATCH v3] fc_sort: avoid compiler warning/error

[Chris PeBenito]

On 06/05/2017 06:08 PM, Guido Trentalancia via refpolicy wrote:
> Fix a "-Werror=implicit-fallthrough" compiler warning/error on
> the switch statement.
>
> This third version (v3) fixes a bug introduced in the first
> version and improves the style over the second version.
>
> Signed-off-by: Guido Trentalancia <guido@trentalancia.com>
> ---
>  support/fc_sort.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> --- a/support/fc_sort.c	2017-06-05 23:49:57.280624866 +0200
> +++ b/support/fc_sort.c	2017-06-06 00:04:48.419621222 +0200
> @@ -292,14 +292,16 @@ void fc_fill_data(file_context_node_t *f
>  			/* If a escape character is found,
>  			 *  skip the next character. */
>  			c++;
> +			break;
>  		default:
> -			/* If no meta character has been found yet,
> -			 *  add one to the stem length. */
> -			if (!fc_node->meta)
> -				fc_node->stem_len++;
>  			break;
>  		}
>
> +		/* If no meta character has been found yet,
> +		 * add one to the stem length. */
> +		if (!fc_node->meta)
> +			fc_node->stem_len++;
> +
>  		fc_node->str_len++;
>  		c++;
>  	}

This doesn't look like a valid change since the block you're doing 
shouldn't be done if the current character is a metacharacter.

-- 
Chris PeBenito

[refpolicy] [PATCH v3] fc_sort: avoid compiler warning/error

[Guido Trentalancia]

Hello. 

My reply follows the quoted message...

On the 7th of June 2017 02:07:49 CEST, Chris PeBenito <pebenito@ieee.org> wrote:
>On 06/05/2017 06:08 PM, Guido Trentalancia via refpolicy wrote:
>> Fix a "-Werror=implicit-fallthrough" compiler warning/error on
>> the switch statement.
>>
>> This third version (v3) fixes a bug introduced in the first
>> version and improves the style over the second version.
>>
>> Signed-off-by: Guido Trentalancia <guido@trentalancia.com>
>> ---
>>  support/fc_sort.c |   10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> --- a/support/fc_sort.c	2017-06-05 23:49:57.280624866 +0200
>> +++ b/support/fc_sort.c	2017-06-06 00:04:48.419621222 +0200
>> @@ -292,14 +292,16 @@ void fc_fill_data(file_context_node_t *f
>>  			/* If a escape character is found,
>>  			 *  skip the next character. */
>>  			c++;
>> +			break;
>>  		default:
>> -			/* If no meta character has been found yet,
>> -			 *  add one to the stem length. */
>> -			if (!fc_node->meta)
>> -				fc_node->stem_len++;
>>  			break;
>>  		}
>>
>> +		/* If no meta character has been found yet,
>> +		 * add one to the stem length. */
>> +		if (!fc_node->meta)
>> +			fc_node->stem_len++;
>> +
>>  		fc_node->str_len++;
>>  		c++;
>>  	}
>
>This doesn't look like a valid change since the block you're doing 
>shouldn't be done if the current character is a metacharacter.

fc_node->stem_len is incremented only if fc_node->meta is zero or, in other words, only if a metacharacter has not been found yet (a condition which does NOT include the case of the current character being a metacharacter). 

So the "if" block that I simply moved out of the "switch" statement, but inside the "while" loop, is NOT executed if the current character is a metacharacter (because in such case fc_node->meta is one and not zero) ! 

I hope this helps.... Please consider that, as it is, the fc_sort code does not compile and the Reference Policy is thus unusable. 

Regards, 

Guido 

[refpolicy] [PATCH v3] fc_sort: avoid compiler warning/error

[Chris PeBenito]

On 06/05/2017 06:08 PM, Guido Trentalancia via refpolicy wrote:
> Fix a "-Werror=implicit-fallthrough" compiler warning/error on
> the switch statement.
>
> This third version (v3) fixes a bug introduced in the first
> version and improves the style over the second version.
>
> Signed-off-by: Guido Trentalancia <guido@trentalancia.com>
> ---
>  support/fc_sort.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> --- a/support/fc_sort.c	2017-06-05 23:49:57.280624866 +0200
> +++ b/support/fc_sort.c	2017-06-06 00:04:48.419621222 +0200
> @@ -292,14 +292,16 @@ void fc_fill_data(file_context_node_t *f
>  			/* If a escape character is found,
>  			 *  skip the next character. */
>  			c++;
> +			break;
>  		default:
> -			/* If no meta character has been found yet,
> -			 *  add one to the stem length. */
> -			if (!fc_node->meta)
> -				fc_node->stem_len++;
>  			break;
>  		}
>
> +		/* If no meta character has been found yet,
> +		 * add one to the stem length. */
> +		if (!fc_node->meta)
> +			fc_node->stem_len++;
> +
>  		fc_node->str_len++;
>  		c++;
>  	}

Merged.

-- 
Chris PeBenito