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

Sdl2Application crash if passed arguments with equal sign (--flag=value) #305

Closed
helmesjo opened this issue Jan 9, 2019 · 3 comments
Closed

Comments

@helmesjo
Copy link

helmesjo commented Jan 9, 2019

So, basically what the title says:
If I pass arguments of the form --flag=value it crashes without any info (not even with debugger attached).
If I pass --flag value it works.

The flag in question is unrelated to magnum (it's used for something else).

Reproduce:

  • Visual Studio 2017 (15.9.4)
  • Debug
  • x86
  • Argument: --flag=value = Fail
  • Argument: --flag value = Success
    (Only tested with above config so far)
struct app : public Magnum::Platform::Application
{
    explicit app(const Arguments& arguments) :
        Magnum::Platform::Application(arguments)
    { }

    virtual void drawEvent() override
    {
    }
};

int main(int argc, char * argv[])
{
    //std::vector<std::string> args(argv + 1, argv + argc);
    app myApp(Magnum::Platform::Sdl2Application::Arguments(argc, argv));
    return 0;
}

Output:
myapp-d.exe: Native' has exited with code -1 (0xffffffff).

@mosra
Copy link
Owner

mosra commented Jan 9, 2019

Thanks for the report!

This is a ... unfortunate combination of corner cases :) The Utility::Arguments class:

  • supports only --flag value, not --flag=value at the moment (it was originally done to make the implementation slightly simpler, but could be easily updated)
  • for magnum *Application classes, all arguments that do not start with --magnum- are ignored (to allow users having their own Utility::Arguments for command-line options and not getting errors about unknown arguments)
  • but only if the argument name is valid (only alphanumeric characters and a -)

So if you would pass e.g. --? there, it would fail too, the same way. I guess the way to fix this is by reverting the order of checks, so first ignore everything non-prefixed and only then check for name validity. Will do that.

What is interesting is that it crashes without any info for you. In my case I get this:

$ magnum-triangle --flag=value
Invalid command-line argument --flag=value
Usage:
  magnum-triangle [--magnum-help] 
    [--magnum-dpi-scaling default|virtual|physical|<d>|"<h> <v>"] 
    [--magnum-disable-workarounds LIST] [--magnum-disable-extensions LIST] 
    [--magnum-log default|quiet|verbose] ...

@mosra mosra added this to the 2019.01 milestone Jan 9, 2019
@mosra mosra added this to TODO in Project management via automation Jan 9, 2019
@mosra mosra self-assigned this Jan 9, 2019
@helmesjo
Copy link
Author

helmesjo commented Jan 9, 2019

Oh, sorry, you are right! I see now that I do get that output (I was just stepping the code and didn't pay attention to the console, only the output in VS 😇 ). But, while debugging it is a dead-hard crash (doesn't even brake).

@mosra
Copy link
Owner

mosra commented Jan 10, 2019

Fixed in mosra/corrade@c11652e, it should not complain anymore but instead ignore the argument completely (as it should be). (It'll be complaining again when you pass e.g. --magnum-blah to it, which is an unknown argument.) I'll be tracking the support for --flag=value (instead of --flag value) separately, so closing this one as resolved.

The Arguments class is calling std::exit() with a non-zero code when it encounters some error in the command-line arguments. It's not a crash, but also not a successful exit, so it can't just return 0. Not sure if the VS debugger can attach to that :)

@mosra mosra closed this as completed Jan 10, 2019
Project management automation moved this from TODO to Done Jan 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

2 participants