Skip to content

Improve handling of special characters in filenames #155

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

Merged
merged 3 commits into from
May 26, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions src/arduino.cc/builder/collect_ctags_from_sketch_files.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ package builder
import (
"arduino.cc/builder/types"
"arduino.cc/builder/utils"
"strings"
)

type CollectCTagsFromSketchFiles struct{}
Expand All @@ -43,7 +42,7 @@ func (s *CollectCTagsFromSketchFiles) Run(ctx *types.Context) error {
allCtags := ctx.CTagsOfPreprocessedSource
ctagsOfSketch := []*types.CTag{}
for _, ctag := range allCtags {
if utils.SliceContains(sketchFileNames, strings.Replace(ctag.Filename, "\\\\", "\\", -1)) {
if utils.SliceContains(sketchFileNames, ctag.Filename) {
ctagsOfSketch = append(ctagsOfSketch, ctag)
}
}
Expand Down
9 changes: 8 additions & 1 deletion src/arduino.cc/builder/ctags/ctags_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,14 @@ func parseTag(row string) *types.CTag {
parts := strings.Split(row, "\t")

tag.FunctionName = parts[0]
tag.Filename = parts[1]
// This unescapes any backslashes in the filename. These
// filenames that ctags outputs originate from the line markers
// in the source, as generated by gcc. gcc escapes both
// backslashes and double quotes, but ctags ignores any escaping
// and just cuts off the filename at the first double quote it
// sees. This means any backslashes are still escaped, and need
// to be unescape, and any quotes will just break the build.
tag.Filename = strings.Replace(parts[1], "\\\\", "\\", -1)
Copy link
Member

Choose a reason for hiding this comment

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

Can we use utils.ParseCppString?

Having a special handling here looks weird, I'd use the full parsing even if ctags doesn't handle \". Or maybe there are corner cases I'm missing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I considered this, but:

  • parseCppString requires double quotes around the string, which should then be added here before passing the string to parseCppString (not really a problem)
  • If a double quote is present in a filename, it is escaped by gcc, but then ctags chops the string at the escaped double quote, causing the filename to end in a backslash. Parsing this by parseCppString will fail, so this would need extra special casing.

Mostly regarding the latter, it seemed better to just handle backslashes here, in a way that also "fixes" any double quotes broken by ctags.

Copy link
Member

Choose a reason for hiding this comment

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

If a double quote is present in a filename, it is escaped by gcc, but then ctags chops the string at the escaped double quote, causing the filename to end in a backslash. Parsing this by parseCppString will fail, so this would need extra special casing.

I see, probably we can just ignore parseCppString errors, and just use the filename as is (parseCppString already return the input filename unchanged if the parsing fails).
In any case a file containing a double quote will not work, but having the full parsing in place may allows future versions of ctags to run properly (without the need to touch this part again).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is true, yes. However, if ctags would properly handle escaping, it would need to unescape quotes and backslashes when reading the line directives, but it would need to escape backslashes and tabs when generating its output, since its fields are currently tab-separated. Alternatively, it would need to add quotes around the filename and escape quotes instead of tabs, meaning that tabs inside quotes are implicitely "escaped" (i.e. are not seen as a field boundary). In either case, arduino-builder needs more changes than you're proposing: splitting the ctags output on tabs will no longer work, since escaped tabs inside the filename should be skipped.

One alternative that would work without major changes with your suggestion is that tabs (and possibly other characters as well) are escaped using octal escapes (e.g. \011 for tab). This is technically supported in a preprocessor string, though gcc line markers do not currently emit them, so utils.parseCppString does not currently parse them either.

Hence I'm inclined to leave this as it is now, and only improve the parsing here if it ever becomes relevant due to ctags improvements (which might not ever happen if we replace ctags soon).


parts = parts[2:]

Expand Down
5 changes: 3 additions & 2 deletions src/arduino.cc/builder/prototypes_adder.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ package builder
import (
"arduino.cc/builder/constants"
"arduino.cc/builder/types"
"arduino.cc/builder/utils"
"fmt"
"strconv"
"strings"
Expand Down Expand Up @@ -87,7 +88,7 @@ func composePrototypeSection(line int, prototypes []*types.Prototype) string {
str := joinPrototypes(prototypes)
str += "\n#line "
str += strconv.Itoa(line)
str += " \"" + prototypes[0].File + "\""
str += " " + utils.QuoteCppString(prototypes[0].File)
str += "\n"

return str
Expand All @@ -99,7 +100,7 @@ func joinPrototypes(prototypes []*types.Prototype) string {
if signatureContainsaDefaultArg(proto) {
continue
}
prototypesSlice = append(prototypesSlice, "#line "+strconv.Itoa(proto.Line)+" \""+proto.File+"\"")
prototypesSlice = append(prototypesSlice, "#line "+strconv.Itoa(proto.Line)+" "+utils.QuoteCppString(proto.File))
prototypeParts := []string{}
if proto.Modifiers != "" {
prototypeParts = append(prototypeParts, proto.Modifiers)
Expand Down
6 changes: 3 additions & 3 deletions src/arduino.cc/builder/sketch_source_merger.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ package builder

import (
"arduino.cc/builder/types"
"arduino.cc/builder/utils"
"regexp"
"strings"
)

type SketchSourceMerger struct{}
Expand All @@ -47,7 +47,7 @@ func (s *SketchSourceMerger) Run(ctx *types.Context) error {
includeSection += "#include <Arduino.h>\n"
lineOffset++
}
includeSection += "#line 1 \"" + strings.Replace((&sketch.MainFile).Name, "\\", "\\\\", -1) + "\"\n"
includeSection += "#line 1 " + utils.QuoteCppString(sketch.MainFile.Name) + "\n"
lineOffset++
ctx.IncludeSection = includeSection

Expand All @@ -73,7 +73,7 @@ func sketchIncludesArduinoH(sketch *types.SketchFile) bool {
}

func addSourceWrappedWithLineDirective(sketch *types.SketchFile) string {
source := "#line 1 \"" + strings.Replace(sketch.Name, "\\", "\\\\", -1) + "\"\n"
source := "#line 1 " + utils.QuoteCppString(sketch.Name) + "\n"
source += sketch.Source
source += "\n"

Expand Down
6 changes: 2 additions & 4 deletions src/arduino.cc/builder/test/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,20 @@ package test
import (
"arduino.cc/builder/constants"
"arduino.cc/builder/types"
"arduino.cc/builder/utils"
"bytes"
"fmt"
"github.com/go-errors/errors"
"github.com/stretchr/testify/assert"
"io/ioutil"
"path/filepath"
"strings"
"testing"
"text/template"
)

func LoadAndInterpolate(t *testing.T, filename string, ctx *types.Context) string {
funcsMap := template.FuncMap{
"EscapeBackSlashes": func(s string) string {
return strings.Replace(s, "\\", "\\\\", -1)
},
"QuoteCppString": utils.QuoteCppString,
}

tpl, err := template.New(filepath.Base(filename)).Funcs(funcsMap).ParseFiles(filename)
Expand Down
73 changes: 37 additions & 36 deletions src/arduino.cc/builder/test/prototypes_adder_test.go

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions src/arduino.cc/builder/test/sketch1/merged_sketch.txt
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
#include <Arduino.h>
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
void setup() {

}

void loop() {

}
#line 1 "{{EscapeBackSlashes (index .sketch.OtherSketchFiles 0).Name}}"
#line 1 {{QuoteCppString (index .sketch.OtherSketchFiles 0).Name}}

#line 1 "{{EscapeBackSlashes (index .sketch.OtherSketchFiles 1).Name}}"
#line 1 {{QuoteCppString (index .sketch.OtherSketchFiles 1).Name}}
String hello() {
return "world";
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include <Arduino.h>
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
#define DEBUG 1
#define DISABLED 0

Expand All @@ -16,17 +16,17 @@ typedef int MyType;

#include "empty_2.h"

#line 16 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 16 {{QuoteCppString .sketch.MainFile.Name}}
void setup();
#line 21 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 21 {{QuoteCppString .sketch.MainFile.Name}}
void loop();
#line 33 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 33 {{QuoteCppString .sketch.MainFile.Name}}
void debug();
#line 44 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 44 {{QuoteCppString .sketch.MainFile.Name}}
void disabledIsDefined();
#line 48 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 48 {{QuoteCppString .sketch.MainFile.Name}}
int useMyType(MyType type);
#line 16 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 16 {{QuoteCppString .sketch.MainFile.Name}}
void setup() {
// put your setup code here, to run once:

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include <Arduino.h>
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
#define DEBUG 1
#define DISABLED 0

Expand Down
10 changes: 5 additions & 5 deletions src/arduino.cc/builder/test/sketch3/Baladuino.preprocessed.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include <Arduino.h>
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
/*
* The code is released under the GNU General Public License.
* Developed by Kristian Lauszus, TKJ Electronics 2013
Expand Down Expand Up @@ -88,11 +88,11 @@ WII Wii(&Btd); // The Wii library can communicate with Wiimotes and the Nunchuck
// This can also be done using the Android or Processing application
#endif

#line 88 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 88 {{QuoteCppString .sketch.MainFile.Name}}
void setup();
#line 204 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 204 {{QuoteCppString .sketch.MainFile.Name}}
void loop();
#line 88 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 88 {{QuoteCppString .sketch.MainFile.Name}}
void setup() {
/* Initialize UART */
Serial.begin(115200);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include <Arduino.h>
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
#include <SoftwareSerial.h> // required to send and receive AT commands from the GPRS Shield
#include <Wire.h> // required for I2C communication with the RTC

Expand Down Expand Up @@ -39,49 +39,49 @@ Code Exclusively for GPRS shield:
// Default set of instructions for GPRS Shield power control
//

#line 39 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 39 {{QuoteCppString .sketch.MainFile.Name}}
void setPowerStateTo( int newState );
#line 64 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 64 {{QuoteCppString .sketch.MainFile.Name}}
int getPowerState();
#line 75 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 75 {{QuoteCppString .sketch.MainFile.Name}}
void powerUpOrDown();
#line 90 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 90 {{QuoteCppString .sketch.MainFile.Name}}
void clearBufferArray();
#line 96 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 96 {{QuoteCppString .sketch.MainFile.Name}}
void makeMissedCall( char num[] );
#line 111 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 111 {{QuoteCppString .sketch.MainFile.Name}}
void sendTextMessage( char number[], char messg[] );
#line 129 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 129 {{QuoteCppString .sketch.MainFile.Name}}
void analise(byte incoming[], int length);
#line 179 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 179 {{QuoteCppString .sketch.MainFile.Name}}
byte decToBcd( byte b );
#line 184 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 184 {{QuoteCppString .sketch.MainFile.Name}}
boolean getBit( byte addr, int pos );
#line 190 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 190 {{QuoteCppString .sketch.MainFile.Name}}
void setBit( byte addr, int pos, boolean newBit );
#line 204 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 204 {{QuoteCppString .sketch.MainFile.Name}}
byte getByte( byte addr );
#line 213 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 213 {{QuoteCppString .sketch.MainFile.Name}}
boolean getBytes( byte addr, int amount );
#line 230 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 230 {{QuoteCppString .sketch.MainFile.Name}}
void setByte( byte addr, byte newByte );
#line 235 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 235 {{QuoteCppString .sketch.MainFile.Name}}
void setBytes( byte addr, byte newBytes[], int amount );
#line 244 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 244 {{QuoteCppString .sketch.MainFile.Name}}
void getTime();
#line 260 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 260 {{QuoteCppString .sketch.MainFile.Name}}
void setTime( byte newTime[ 7 ] );
#line 267 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 267 {{QuoteCppString .sketch.MainFile.Name}}
void getRTCTemperature();
#line 277 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 277 {{QuoteCppString .sketch.MainFile.Name}}
void gprsListen();
#line 294 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 294 {{QuoteCppString .sketch.MainFile.Name}}
void printTime();
#line 317 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 317 {{QuoteCppString .sketch.MainFile.Name}}
void setup();
#line 334 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 334 {{QuoteCppString .sketch.MainFile.Name}}
void loop();
#line 39 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 39 {{QuoteCppString .sketch.MainFile.Name}}
void setPowerStateTo( int newState )
{
if( newState != 1 && newState != 0 ) { // tests for an invalid state. In this case no change is made to powerstate
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
#include <Arduino.h>
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
#include <CapacitiveSensor.h>
/*
#include <WiFi.h>
*/
CapacitiveSensor cs_13_8 = CapacitiveSensor(13,8);
#line 6 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 6 {{QuoteCppString .sketch.MainFile.Name}}
void setup();
#line 10 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 10 {{QuoteCppString .sketch.MainFile.Name}}
void loop();
#line 6 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 6 {{QuoteCppString .sketch.MainFile.Name}}
void setup()
{
Serial.begin(9600);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
#include <Arduino.h>
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
const char *foo = "\
hello \
world\n";

//" delete this comment line and the IDE parser will crash

#line 7 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 7 {{QuoteCppString .sketch.MainFile.Name}}
void setup();
#line 11 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 11 {{QuoteCppString .sketch.MainFile.Name}}
void loop();
#line 7 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 7 {{QuoteCppString .sketch.MainFile.Name}}
void setup()
{
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
#include <Arduino.h>
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
void setup();
#line 10 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 10 {{QuoteCppString .sketch.MainFile.Name}}
void loop();
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
void setup() {
// put your setup code here, to run once:
// "comment with a double quote
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include <Arduino.h>
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
/* START CODE */

struct A_NEW_TYPE {
Expand All @@ -9,13 +9,13 @@ struct A_NEW_TYPE {
int c;
} foo;

#line 9 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 9 {{QuoteCppString .sketch.MainFile.Name}}
void setup();
#line 13 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 13 {{QuoteCppString .sketch.MainFile.Name}}
void loop();
#line 17 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 17 {{QuoteCppString .sketch.MainFile.Name}}
void dostuff (A_NEW_TYPE * bar);
#line 9 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 9 {{QuoteCppString .sketch.MainFile.Name}}
void setup() {

}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include <Arduino.h>
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
#line 1 {{QuoteCppString .sketch.MainFile.Name}}
#include "config.h"

#ifdef DEBUG
Expand All @@ -13,11 +13,11 @@

#include <Bridge.h>

#line 13 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 13 {{QuoteCppString .sketch.MainFile.Name}}
void setup();
#line 17 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 17 {{QuoteCppString .sketch.MainFile.Name}}
void loop();
#line 13 "{{EscapeBackSlashes .sketch.MainFile.Name}}"
#line 13 {{QuoteCppString .sketch.MainFile.Name}}
void setup() {

}
Expand Down
Loading