Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix include handle #2600

Closed
wants to merge 3 commits into from
Closed

Conversation

877509395
Copy link

"const char *file = strchr(yytext, ' ') + 1" supposes only one space after Include, but the grammar "[ \t]+" mean multiple space and TAB including one space;

@martinhsv
Copy link
Contributor

martinhsv commented Aug 10, 2021

Just to flesh out a little background (and correct me if you think I'm mistaken) ...

In v3.0.3, several configuration directives allowed for exactly one space character after the directive name.

v3.0.4 included changes ( #2006 ) to broaden the whitespace that is allowed for many of these directives, including the two referenced in this pull request.

The last two changes in that commit ( dc78c0e ) changed the whitespace definition from '[ ]' to '[ \t]+' ...

... but that that portion of change appears to be incomplete.

That seems to me to be a correct conclusion. For one thing, using tabs instead of spaces after the 'Include' keyword is a problem use case.

Turning to this proposed fix:

One thing I'm a little wary of, is including regex.h here. We haven't previously used that within the parser subcomponent, and, given that Flex has its own regular expression engine, it might be confusing in future if we start mixing that in.

@877509395
Copy link
Author

I will remove regex.h;

@877509395
Copy link
Author

877509395 commented Aug 11, 2021

Yes, You are right
If no #2006 included, use one space is correct;
but after #2006 included, [ \t]+ should be supported correctly for {CONFIG_INCLUDE}[ \t]+["]{CONFIG_VALUE_PATH}[" and {CONFIG_INCLUDE}[ \t]+{CONFIG_VALUE_PATH}.
So after v3.0.4, it should be fixed, using tabs/spaces after the 'Include' keyword should not be a problem of use case, the code and the lex do not match.

root and others added 2 commits August 10, 2021 22:08
…ce after Include, but the grammar "[ \t]+" mean multiple space/TAB including one space;

2. "f[strlen(f)-1] = '\0'" is extra code after strdup.
@877509395
Copy link
Author

ignore this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants