On Thu, Oct 14, 2010 at 3:44 PM, Dan McGee <dpmcgee@gmail.com> wrote:
Is the assert stuff you added to be safer? We don't want to use this. One, they are compiled into nothing if NDEBUG is defined, and two, it is a bit silly to abort on this condition. cmp(NULL, NULL) ==0, and then just decide whether to sort NULL first or last.
It was not supposed to be safer, just to explicitly state that this case should not happen. If gettext returns NULL, our --help output is going to look awesome : (null)(null)(null).. It's not perfectly clear to me that an assert is worse. In the unlikely case that this could be triggered, at least an assert would fail gracefully telling us where it failed and possibly giving a core dump that would tell us more about what happened. Anyway we could debate over and over about assert usage. Maybe it's more interesting for an expensive check in a hot-path that you do not want to keep in a release build, but you want to make sure that the assert is indeed not triggered while developing / testing. I updated the patch with an explicit check (I actually already did this before choosing to use assert) : http://code.toofishes.net/cgit/xavier/pacman.git/log/?h=parseargs