[apparmor] [PATCH] libapparmor: Fix -Wunused-but-set-variable GCC warning

[Tyler Hicks]

On 2016-01-29 14:51:53, Seth Arnold wrote:
> On Fri, Jan 29, 2016 at 03:18:37PM -0600, Tyler Hicks wrote:
> > Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
> 
> While I don't see anything wrong with this patch I can't actually figure
> out how this function works anyway. I don't see anything that would ever
> update the ret_record structure after it's been initialized, and I can't
> find the aalogparse_parse() function at all. (It feels a bit like it'd be
> a magically constructed macro but I can't even find the macros that would
> construct it.)
> 
> So while this patch may silence a valid warning, I'm not sure how this
> works anyway.
> 
> Did I miss something?

No, it is confusing and I dug deep enough to figure out that it is
working as intended. Here's what I know (requires building libapparmor):

libraries/libapparmor/src/grammar.c:
    62  /* Substitute the variable and function names.  */
    63  #define yyparse         aalogparse_parse
    64  #define yylex           aalogparse_lex
    65  #define yyerror         aalogparse_error
    66  #define yydebug         aalogparse_debug
    67  #define yynerrs         aalogparse_nerrs

libraries/libapparmor/src/grammar.y:
    32  /* Since we're a library, on any errors we don't want to print out any
    33   * error messages. We should probably add a debug interface that does
    34   * emit messages when asked for. */
    35  void aalogparse_error(void *scanner, char const *s)
    36  {
    37          //printf("ERROR: %s\n", s);
    38          ret_record->event = AA_RECORD_INVALID;
    39  }

So aalogparse_parse is yyparse and aalogparse_error is yyerror.

I originally patched grammar.y to honor errors returned from
aalogparse_parse() and return NULL. When I ran the `make check`, it
failed because tests like
libraries/libapparmor/testsuite/test_multi/avc_audit_invalid_audit_id.out
ensure that an AA_RECORD_INVALID event is returned from
yyparse/aalogparse_parse instead of an error.

That's when I decided that the yyparse/aalogparse_parse return code is
intentionally ignored and (void)ed it. Doing anything else would change
the libaalogparse behavior when it encounters invalid events.

Tyler

> 
> Thanks
> 
> > ---
> >  libraries/libapparmor/src/grammar.y | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libraries/libapparmor/src/grammar.y b/libraries/libapparmor/src/grammar.y
> > index 2a16d72..1dcc2f9 100644
> > --- a/libraries/libapparmor/src/grammar.y
> > +++ b/libraries/libapparmor/src/grammar.y
> > @@ -425,7 +425,6 @@ _parse_yacc(char *str)
> >  	/* yydebug = 1;  */
> >  	YY_BUFFER_STATE lex_buf;
> >  	yyscan_t scanner;
> > -	int parser_return;
> >  
> >  	ret_record = NULL;
> >  	ret_record = malloc(sizeof(aa_log_record));
> > @@ -441,7 +440,8 @@ _parse_yacc(char *str)
> >  
> >  	aalogparse_lex_init(&scanner);
> >  	lex_buf = aalogparse__scan_string(str, scanner);
> > -	parser_return = aalogparse_parse(scanner);
> > +	/* Ignore return value to return an AA_RECORD_INVALID event */
> > +	(void)aalogparse_parse(scanner);
> >  	aalogparse__delete_buffer(lex_buf, scanner);
> >  	aalogparse_lex_destroy(scanner);
> >  	return ret_record;



> -- 
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20160129/fe636133/attachment.pgp>