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

Pascal parser: ensure TagEntryInfo is always initialized #2987

Closed
wants to merge 1 commit into from

Conversation

eht16
Copy link
Member

@eht16 eht16 commented Nov 6, 2021

This fixes crashes when using incomplete "type" declarations.

Fixes #2358, fixes #2982 and fixes #2428.

The initTagEntry() call is necessary to initialize the TagEntryInfo object. The KIND_GHOST_INDEX argument is taken from the uctags Pascal parser.

@@ -51,7 +51,7 @@ static void createPascalTag (tagEntryInfo* const tag,
else
{
/* TODO: Passing NULL as name makes an assertion behind initTagEntry failure */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not so sure about this comment but I left it as it is also still present in the uctags Pascal parser.

@elextr
Copy link
Member

elextr commented Nov 6, 2021

Is this fixed in #2984?

@eht16
Copy link
Member Author

eht16 commented Nov 6, 2021

Is this fixed in #2984?

Not sure. I guess not as it doesn't contain parser changes.

@elextr
Copy link
Member

elextr commented Nov 6, 2021

Ok, good, that also means it won't undo it when #2984 is merged.

This fixes crashes when using incomplete "type" declarations.

Fixes geany#2358, fixes geany#2982 and fixes geany#2428.
@eht16 eht16 force-pushed the issue2358_fix_pascal_parser_crash branch from 4d0874e to bf6c1f7 Compare November 7, 2021 10:08
@eht16
Copy link
Member Author

eht16 commented Nov 7, 2021

Tested the problematic type declaration against #2984 without this change and it still crashes.
Tested the problematic type declaration against #2984 with this change and it still works.

Also added another example declaration to the test case for better coverage (and it triggered Travis CI which is working again).

@eht16
Copy link
Member Author

eht16 commented Nov 25, 2021

@techee what do you think? Should we consider merging this or rather handle it in #2991 (or a following PR)?

@techee
Copy link
Member

techee commented Nov 25, 2021

@techee what do you think? Should we consider merging this or rather handle it in #2991 (or a following PR)?

Has this change been submitted upstream so it doesn't get lost?

Basically this depends on the outcome of the discussion in #2991 (do you have any feedback for that one?). Pascal is one of the languages we could probably take from uctags and if we do (and this change isn't there yet), this patch should wait until #2991 is merged, otherwise it gets overwritten.

@eht16
Copy link
Member Author

eht16 commented Nov 28, 2021

@techee what do you think? Should we consider merging this or rather handle it in #2991 (or a following PR)?

Has this change been submitted upstream so it doesn't get lost?

The change in the parser itself is actually taken from uctags.

Basically this depends on the outcome of the discussion in #2991 (do you have any feedback for that one?). Pascal is one of the languages we could probably take from uctags and if we do (and this change isn't there yet), this patch should wait until #2991 is merged, otherwise it gets overwritten.

#2991 (comment).

eht16 added a commit to eht16/geany that referenced this pull request Dec 5, 2021
@techee
Copy link
Member

techee commented Dec 5, 2021

@eht16 I guess this pull request is superseded by #3043 and can be closed, right? (Just trying to keep track of all the parser pull requests we have open.)

@eht16
Copy link
Member Author

eht16 commented Dec 6, 2021

@techee yes, that's the plan. This PR should be closed automatically when #3043 is merged.

eht16 added a commit to eht16/geany that referenced this pull request Dec 8, 2021
eht16 added a commit to eht16/geany that referenced this pull request Dec 11, 2021
@techee techee closed this in #3043 Dec 28, 2021
@eht16 eht16 deleted the issue2358_fix_pascal_parser_crash branch January 9, 2022 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants