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

tagmanager: do not rewind file on '# format=ctags' #1816

Closed
wants to merge 1 commit into from

Conversation

lpaulsen93
Copy link
Contributor

On import of a global tags file, do not rewind the file if the first line is # format=ctags. Otherwise the import would abort on the first line and 0 symbols would be imported. Fixes #1814.

On import of a global tags file, do not rewind the file if the first line is '# format=ctags'.
Otherwise the import would abort on the first line and 0 symbols would be imported.
Fixes geany#1814.
@elextr
Copy link
Member

elextr commented Mar 22, 2018

No, that won't work. Simply not rewinding will leave the file having read BUFSIZ which is guaranteed to be at least 256, so well past the end of the type spec that is to be skipped. So have to always rewind, then forward over the # format=ctags part (probably keep the length from having found it).

Or I'm not sure why it doesn't use getline() instead of reading BUFSIZ, but anyway.

Also noticed the "skip" code here would seem to actually be an infinite loop?

[Edit: So when its fixed, maybe it can be changed to skip # format=ctags as well.]

@lpaulsen93
Copy link
Contributor Author

No, fgets also stops at a newline, see this cite from cplusplus.com:

Reads characters from stream and stores them as a C string into str until (num-1) characters have been read or either a newline or the end-of-file is reached, whichever happens first.

A problem would only occur if the line is longer than ``BUFSIZ```.

@elextr
Copy link
Member

elextr commented Mar 22, 2018

No, fgets also stops at a newline, see this cite from cplusplus.com:

Ok.

b4n added a commit to b4n/geany that referenced this pull request Mar 23, 2018
This prevents loading a spurious tag for the format specifier line, as
well as fixing loading of CTags format with a format specifier line.

Before this change, the file pointer was rewound after reading a format
specifier line; but this had various unwanted side effects depending on
the recognized format:

* For TagManager and Pipe formats, it led to loading a tag named after
  the format specifier (e.g. a literal "# format=tagmanager").  This
  was fairly harmless and only introduced a spurious tag seldom even
  used because "#" isn't usually considered for looking up completions.
* For CTags format, having an explicit specifier led to failure to load
  the file in most cases because the specifier line would be parsed but
  doesn't usually follow the format's requirements, leading to early
  abortion loading that file.  On some very specific specifier lines
  actually following CTags format, it could have led to loading a
  spurious tag instead.

Fixes geany#1814 and closes geany#1816.
@b4n
Copy link
Member

b4n commented Mar 23, 2018

While this seems to be a fair enough fix for the problem you describe, there's actually more issues with this rewind() call, although they are a lot less important indeed. See #1817.

@lpaulsen93
Copy link
Contributor Author

Ok, closing this.

@lpaulsen93 lpaulsen93 closed this Mar 23, 2018
@b4n b4n added this to the 1.34 milestone Nov 30, 2018
@lpaulsen93 lpaulsen93 deleted the issue1814 branch October 15, 2019 17:28
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.

None yet

3 participants