Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf
From: Jeff King <hidden>
Date: 2016-06-15 23:06:34
On Tue, Sep 15, 2015 at 01:38:42PM -0700, Stefan Beller wrote:
Some off topic thoughts: Having an "assert" behavior is not a good user experience though and should be fixed. To fix it we need stack traces. And the git version. And telling the user to send it to the mailing list.
Yes, any assert (or die("BUG")) should generally result in a code change
to make it never happen again.
I have been tempted to replace our die("BUG") calls with a BUG() macro,
so that we can do more fancy things (even if it is just adding
__FILE__:__LINE__ and a message to contact the list).
I wonder if we want to include a trace where possible (i.e. when compiled with gcc or other precompiler conditions) into these assertive behaviors.
That would be nice. I took a look at this back in April: http://thread.gmane.org/gmane.comp.version-control.git/267643/focus=267755 but I think I convinced myself that we are better off relying on people running gdb. If we do anything, it should be to make doing so easier.
I'd guess we have an assertive behavior if die("BUG:...") is called,
so maybe we can just check inside of die if we want to print
a stack trace additionally ?I guess we could look for starts_with(fmt, "BUG: ") in die(). I think it would be a bit less gross to convert those calls to a BUG() macro (we can't get accurate __LINE__ information without it, though in practice the die messages are usually unique enough to be helpful). I think a core file is even more useful than a backtrace. Having BUG() call abort() would be even more useful, I think. -Peff