Skip to content

ctags: filterSketchSource: enlarge buffer size to handle long lines #2935

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

ctags: filterSketchSource: enlarge buffer size to handle long lines #2935

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions internal/arduino/builder/internal/preprocessor/ctags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
}

if src, err := ctagsTarget.ReadFile(); err == nil {
filteredSource := filterSketchSource(sketch, bytes.NewReader(src), false)
filteredSource := filterSketchSource(sketch, bytes.NewReader(src), false, stderr)
if err := ctagsTarget.WriteFile([]byte(filteredSource)); err != nil {
return &Result{args: result.Args(), stdout: stdout.Bytes(), stderr: stderr.Bytes()}, err
}
Expand Down Expand Up @@ -208,7 +208,7 @@
return stdout, stderr, args, err
}

func filterSketchSource(sketch *sketch.Sketch, source io.Reader, removeLineMarkers bool) string {
func filterSketchSource(sketch *sketch.Sketch, source io.Reader, removeLineMarkers bool, stderr *bytes.Buffer) string {
fileNames := paths.NewPathList()
Comment on lines +211 to 212
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func filterSketchSource(sketch *sketch.Sketch, source io.Reader, removeLineMarkers bool, stderr *bytes.Buffer) string {
fileNames := paths.NewPathList()
func filterSketchSource(sketch *sketch.Sketch, source io.Reader, removeLineMarkers bool) (string, error) {
fileNames, filterErr := paths.NewPathList()
if filterErr != nil {
if errors.Is(filterErr, bufio.ErrTooLong) {
fmt.Fprintf(stderr, "%s: %s",
i18n.Tr("An error occurred adding prototypes"),
i18n.Tr("line too long\n"))
}
return &Result{args: result.Args(), stdout: stdout.Bytes(), stderr: stderr.Bytes()}, filterErr
}

If missing the long line is indeed problematic, we should treat it as an error, instead of writing to stderr but still moving forward.

Copy link
Member Author

Choose a reason for hiding this comment

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

The best solution would be to just skip the line, since we are only interested in the first few bytes of the ones starting with #. However, this would require getting rid of the Scanner entirely and replacing it with some manual read.

fileNames.Add(sketch.MainFile)
fileNames.AddAll(sketch.OtherSketchFiles)
Expand All @@ -217,6 +217,11 @@
filtered := ""

scanner := bufio.NewScanner(source)

var buf []byte
const maxTokenSize = 1024 * 1024 // 1 MB
scanner.Buffer(buf, maxTokenSize)

for scanner.Scan() {
line := scanner.Text()
if filename := cpp.ParseLineMarker(line); filename != nil {
Expand All @@ -230,6 +235,11 @@
filtered += line + "\n"
}
}
if scanner.Err() == bufio.ErrTooLong {

Check failure on line 238 in internal/arduino/builder/internal/preprocessor/ctags.go

View workflow job for this annotation

GitHub Actions / check-style (./) 

comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)
fmt.Fprintf(stderr, "%s: %s",
i18n.Tr("An error occurred adding prototypes"),
i18n.Tr("line too long\n"))
}

return filtered
}
Loading