retarget: Fix path behaviour without leading slash #6156
Conversation
Current behaviour ends up undefined when the constructor leaves early. Now FilePath just discards leading slashes and otherwise asserts if the path is bad.
|
|
||
| const char* file_system = &file_path[1]; | ||
| const char* file_system = file_path; |
There was a problem hiding this comment.
Out of curiosity, why is this a const char* instead of char*?
There was a problem hiding this comment.
Because file_path is a const char*. To change to char * would break constness. But this is describing the data, not the pointer itself. If you wanted a const pointer it'd look like this const char *const file_path, and then you would be prevented from modifying the pointer itself. Isn't it beautiful?
| @@ -18,9 +18,10 @@ | |||
| namespace mbed { | |||
|
|
|||
| FilePath::FilePath(const char* file_path) : file_name(NULL), fb(NULL) { | |||
| if ((file_path[0] != '/') || (file_path[1] == 0)) return; | |||
| // skip slashes | |||
| file_path += strspn(file_path, "/"); | |||
There was a problem hiding this comment.
That's an amusing way to catch a slew of slashes.
There was a problem hiding this comment.
What's more amusing is the rest of the function could be replaced by:
len = strcspn(file_path, "/");
|
/morph build |
Build : SUCCESSBuild number : 1210 Triggering tests/morph test |
Exporter Build : SUCCESSBuild number : 881 |
Test : FAILUREBuild number : 1010 |
|
aborted build, restarting /morph test |
Test : FAILUREBuild number : 1011 |
|
Looking at the logs, seems this change is causing one target to timeout, please review if it's related or we are having target/network issue, will check other test results if we can reproduce this somewhere else This is the last output from the device: |
|
/morph test |
Test : SUCCESSBuild number : 1023 |
Current behaviour ends up undefined when the constructor leaves early. Now FilePath just discards leading slashes and otherwise asserts if the path is bad.
Note: The correct behavior should be to return ENOENT in this case, but in order to propagate errors this layer would require quite a bit of restructuring. Since this is currently blocking #5959 and assert is a good compromise.
related #5959, #6090
release patch
cc @deepikabhavnani, @kegilbert