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

Enhancement: A way to get the parser error output from ParseCommandLineFlags #53

Open
GoogleCodeExporter opened this issue Mar 13, 2015 · 5 comments

Comments

@GoogleCodeExporter
Copy link

When encountering an unrecognized flag in ParseCommandLineFlags, ReportError 
writes directly to stderr and then calls exit(1) (through gflags_exitfunc). 
This behavior does not work for any application where stderr is not visible 
(or, in my case, needs to end up in syslog).

Workaround:
I set google::gflags_exitfunc to point at my own function, which records if it 
was called and temporarily dup stderr to a pipe. In cases where 
ParseCommandLineFlags would have resulted in an exit(1) call, I read data from 
the pipe and push it into syslog and then exit(1).

Philosophy:
My workaround is fine, but this seems like quite a useful feature to have.

Original issue reported on code.google.com by [email protected] on 8 May 2012 at 8:06

@GoogleCodeExporter
Copy link
Author

Thanks, Travis, for this proposal and description of your scenario. As it seems 
like the gflags_exitfunc hook was not even intended for external use, only for 
use by the unit tests of the gflags library itself. Your work-around to 
overcome this lack of interface to handle errors yourself, is certainly the 
best you could do.

Apparently, ReportError() is called with either DIE or DO_NOT_DIE whenever an 
error occurred. In most cases, this function is called with DIE as usually when 
a program cannot parse it's command-line arguments you would expect it to exit 
right away.

In order to provide an interface for handling errors, the solution with the 
least impact is to enable the user to specify their own implementation of 
ReportError(). Letting users handle all errors if they desire would also enable 
the handling of the error that occurs when a flag value is set from the value 
of an environment variable. Right now, if that environment variable is set 
incorrectly, the application will die on startup. Instead, it could actually be 
more graceful in certain cases and just fall back to the default value.

What do you think if we add a function pointer such as

   bool (*ReportError)(const char* flag, const char* format, ...);

where

flag - Name of flag which failed validation, could not be parsed, or NULL 
otherwise.
format - Error format string.

and if the function returns true, gflags shall call the exit() function (for 
testing purposes, i.e., yet gflags_exitfunc), otherwise, it will just continue 
if possible. On fatal errors, e.g., those occurring during RegisterFlag(), 
gflags might simply ignore the return value of ReportError() and exit in any 
case.

Original comment by [email protected] on 10 May 2012 at 3:18

  • Added labels: Type-Enhancement
  • Removed labels: Type-Defect

@GoogleCodeExporter
Copy link
Author

Argh, I wanted to check if it is even possible to have a function pointer like 
this, i.e., one with variable arguments... also for convenience of the user, it 
would probably be better to use

    bool (*OnFlagError)(const char* flag, const char* errmsg);

instead and wrap the call to this function internally in another function such 
as

void ReportError(const char* flag, const char* format, ...)
{
     // format error message in store it in errmsg
     if (OnFlagError(flags, errmsg)) gflags_exitfunc(1);
}

Original comment by [email protected] on 10 May 2012 at 3:25

@GoogleCodeExporter
Copy link
Author

Alright, so the solution to the va_list dilemma is described at 
http://c-faq.com/varargs/handoff.html ... the advantage would be the possible 
use of vfprintf() in which case we would not need to allocate a fixed string 
buffer.

New suggestion for the callback function:

     bool (*OnFlagError)(const char* flag, const char* fmt, va_list args);

Original comment by [email protected] on 10 May 2012 at 3:45

@GoogleCodeExporter
Copy link
Author

Original comment by [email protected] on 20 Mar 2014 at 4:13

  • Changed state: Accepted
  • Added labels: Milestone-v2.2

@GoogleCodeExporter
Copy link
Author

Wow, sorry I forgot to check up on this issue for 2 years...things got busy :-)

Anyway, your solution seems awesome to me. Thanks a bunch!

Original comment by [email protected] on 20 Mar 2014 at 5:38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant