[apparmor] [PATCH 01/10] clean up the lexer

[John Johansen]

On 07/24/2013 12:33 AM, Seth Arnold wrote:

<< snip >>

>>  <SUB_VALUE>{
>> -	({IDS}|{QUOTED_ID})	{
>> -			  /* Ugh, this is a gross hack. I used to use
>> -			   * {IDS} to match all TOK_IDs, but that would
>> -			   * also match TOK_MODE + TOK_END_OF_RULE
>> -			   * without any spaces in between (because it's
>> -			   * a longer match). So now, when I want to
>> -			   * match any random string, I go into a
>> -			   * separate state. */
>> -			DUMP_PREPROCESS;
>> -			yylval.id =  processid(yytext, yyleng);
>> -			PDEBUG("Found sub value: \"%s\"\n",  yylval.id);
>> -			yy_pop_state();
>> -			return TOK_VALUE;
>> -		}
>> -
>> -	[^\n]	{
>> -			DUMP_PREPROCESS;
>> -			/* Something we didn't expect */
>> -			yyerror(_("Found unexpected character: '%s'"), yytext);
>> -		}
>> +	({IDS}|{QUOTED_ID}) {
>> +		/* Go into separate state to match generic VALUE strings */
>> +		yylval.id =  processid(yytext, yyleng);
>> +		POP_AND_RETURN(TOK_VALUE);
>> +	}
>>  }
> 
> Can <SUB_ID> and <SUB_VALUE> be combined here? Is the clarity increased
> if they are combined?
> 

maybe, but not in this patch. I need to look at reworking how ids and
values are used in parser_yacc.y

<< snip >>

>>  
>> -	{END_OF_RULE}	{
>> -			DUMP_PREPROCESS;
>> -			yylval.id = strdup(yytext);
>> -			yyerror(_("Variable declarations do not accept trailing commas"));
>> -			}
>> +	{END_OF_RULE} {
>> +		yylval.id = strdup(yytext);
>> +		DUMP_PREPROCESS;
>> +		yyerror(_("Variable declarations do not accept trailing commas"));
>> +	}
> 
> It wasn't introduced here, but I don't understand the strdup(),
> yyerror() is going to exit anyway.
> 

well I'm not going to change it here either, see I have this long term
plan to turn this mess into a library, at which point it won't be
exiting

<< snip >>
 
>>  <RLIMIT_MODE>{
>> -	{WS}+		{ DUMP_PREPROCESS; /* Eat whitespace */ }
>> -
>> -
>>  	-?{NUMBER}[[:alpha:]]*  {
> 
> Not introduced in this patch, but can we use a more-specific set of
> chars here to give better error messages? Or would errors get worse? At
> least line number is easily available here...
> 
nope, I am keeping the lexer wide, and trying to get to having more
specific errors out of the parser and semantic check stages

<< snip >>

>> +#include/.*\r?\n	{
> 
> Hunh, I don't think I knew that "# include" wouldn't include a file. Now I
> do know. :)
> 

yep its a comment and one of the differences between our include and the C
preprocessor.  Really #include was a very, very, very bad choice for our
includes, as we where using # for comments.

Thats why I added support for the bare include keyword years ago. Sadly
it seems to never get used in policy.

But I see a patch coming on :)


thanks Seth