Fails to properly split `x=\"hello world\" -something=else`

[Disservin]

Fails to properly split `x=\"hello world\" -something=else`

[bitmeal]

I cannot reproduce the issue.
Could you please provide a full reproducible example how you get these results

analysis

MWE

Expanding your code sample to a full program github_issue_2.cpp:

# github_issue_2.cpp

#include <stdlib.h>
#include <iostream>

#include "argv_split.h"

int main(int _argc, char** _argv)
{
        auto argv = argv_split("test");
        argv.parse("x=\"hello world\" -something=else");

        std::cout << argv.argv()[0] << std::endl;
        std::cout << argv.argv()[1] << std::endl;
        std::cout << argv.argv()[2] << std::endl;

        return EXIT_SUCCESS;
};

Use CMake and minimal CMakeLists.txt:

# CMakeLists.txt
add_executable(github_issue_2 github_issue_2.cpp)

Build (for me, using NMake Makefiles and MSVC compiler).

cmake -G="NMake Makefiles" --build build
cmake --build build

Run:

$ .\build\github_issue_2.exe
test
x=hello world
-something=else

ctest

Adding your example to the tests in test/CMakeLists.txt; append line:

add_cmdline_test(github_issue_2 "x=\"hello world\" -something=else")

Yields testing log:

Command: "<REDACTED>/github_issue_2.exe" "x=hello world" "-something=else"
Directory: <REDACTED>
"github_issue_2_test" start time: Apr 05 13:11 W. Europe Daylight Time
Output:
----------------------------------------------------------
github_issue_2
x=hello world
-something=else

argc: 3
<REDACTED>/github_issue_2.exe
x=hello world
-something=else

<end of output>
Test time =   0.05 sec
----------------------------------------------------------
Test Passed.
"github_issue_2_test" end time: Apr 05 13:11 W. Europe Daylight Time
"github_issue_2_test" time elapsed: 00:00:00
----------------------------------------------------------

[Disservin]

https://godbolt.org/z/861o8ebj4

Only happens with GCC and -O3 (actually -O1), clang is fine... perhaps a compiler miscompilation?
Last working compiler version for GCC was 7.5

[Disservin]

If you don't see any undefined behaviour in your lib, then I'll open an issue with gcc.

No compiler flags nor valgrind showed any potential problems for me (after applying my fixes from #1), though this weird behaviour can be reproduced on master.

[Disservin]

You have some undefined behaviour here:
https://github.com/bitmeal/argv_split/blob/master/argv_split.h#L53

https://cplusplus.com/reference/string/string/find_first_of/

s
Pointer to an array of characters.
If argument n is specified (3), the first n characters in the array are searched for.
Otherwise (2), a null-terminated sequence is expected: the length of the sequence with the characters to match is determined by the first occurrence of a null character.

Your code is using the second signature which expects a null terminated sequence, however you only define an array with size 1, which is not a valid c string because it has no null terminator..

find_first_of has a signature for just c, just replace

char match[1];
match[0] = str.at( match_idx );

with

char match = str.at( match_idx );

or add 1 to the parameter list of the function call to find_first_of or makem the array the right size.

[bitmeal]

Thank you for the analysis! Could you provide your changes as a pull request?

Currently I am thinking about a complete rewrite in more concise and clear to follow code. Though I have not made any use of this project in a long time myself.

[Disservin]

Thank you for the analysis! Could you provide your changes as a pull request?

Sure I can do that later.

Currently I am thinking about a complete rewrite in more concise and clear to follow code. Though I have not made any use of this project in a long time myself.

I've switched to using wordexp on linux for now https://man7.org/linux/man-pages/man3/wordexp.3.html. On Windows I don't need this functionality but the wordexp use is a bit ugly so a simple c++ parser would be much cleaner I think.

complete rewrite in more concise and clear to follow code.

Yeah I've seen that the last commit is some 6 years old, I was quite surprised that you even answered this :D

[bitmeal]

I've switched to using wordexp on linux for now https://man7.org/linux/man-pages/man3/wordexp.3.html. On Windows I don't need this functionality but the wordexp use is a bit ugly so a simple c++ parser would be much cleaner I think.

You may want to check out a new implementation I put together: https://github.com/bitmeal/argv_split/tree/algo_tokens

[Disservin]

That looks already better.

1. both readmes have argv_parser instead of argv_split in their docs
2. extra semicolon on line 116 with the new version

argv_split.h:116:10: warning: extra ‘;’ [-Wpedantic]
  116 |         };

3. This string

curl -X "POST" "https://echo.paw.cloud/" -H 'Accept: application/json' -H 'Content-Type: text/plain; charset=utf-8' -d "foo:\"bar\""

is parsed like this

curl
-X
"POST"
"https://echo.paw.cloud/"
-H
'Accept: application/json'
-H
'Content-Type: text/plain; charset=utf-8'
-d
"foo:\"bar\""

looks correct except the ' around the header parts?

[Disservin]

Opened a pr for the UB fix, might be useful to have it still even if there is perhaps a better branch available now.

[bitmeal]

That looks already better.

curl
-X
"POST"
"https://echo.paw.cloud/"
-H
'Accept: application/json'
-H
'Content-Type: text/plain; charset=utf-8'
-d
"foo:\"bar\""

looks correct except the ' around the header parts?

On a second thought, this does not look good at all! All enclosing quotes are still present; this is not how it should be. "minor" oversight... Interestingly, tests on Windows did pass. This may require more evaluation of command line argument parsing on Windows.

I would close this issue for now, as the issue itself was fixed with your PR #3. As your goal was having a parser working on Win and Nix - as is mine - I will report back when I get the time fixing the new variant.

[Disservin]

On a second thought, this does not look good at all! All enclosing quotes are still present; this is not how it should be. "minor" oversight... Interestingly, tests on Windows did pass. This may require more evaluation of command line argument parsing on Windows.

I'm sorry somehow my escape strings got lost on the "way", this was actually my string. Here it is correct that you see the " since those are escaped, only ' is incorrect.

curl -X \"POST\" \"https://echo.paw.cloud/\" -H 'Accept: application/json' -H 'Content-Type: text/plain; charset=utf-8' -d 'foo:\\\"bar\\\"'