2010/1/5 Charles Wilson: > Personally, I dislike global static variables.
There may be good reasons to avoid global variables that are shared between different source files, and they should certainly be treated with utmost suspicion in multithreaded code. But in a small single-threaded one-file program, I think this is superstition that does nothing but clutter up the program and introduce opportunity for errors. Passing around stderr is particularly silly, as that won't make the global variable go away. > (Not that any of the cygutils tools do this; so if you want to globally > modify all cygutils popt-using utilities to make optCon a static var, I > would accept that patch; but let's do it separately Fair enough. Then again, I'd be ok to accept this as a matter of opinion anyway and add the (un)necessary arguments to the newly introduced helper functions. > I think that, in general, --help output should go to stdout, not stderr. > Also --usage, --license, and --version output. However, if an *error* > is handled by printing the usage(), then it should go to stderr. > That wasn't the case in mkshortcut. [...] Let's punt on this, too, and I'll do it all-at-once. Makes sense, and I'm certainly happy to leave that to you. > Finally, what you call "pointless freeing" I call "good coding > practice". Sure, in each case the very next thing that will happen is > exit() or abort(), so the system will reclaim that memory immediately. > So these "leaks" are harmless -- but they ARE leaks, and > valgrind/duma/purify will report them as such. I'm shocked you're serious about this, and couldn't disagree more. These cases are not leaks in any meaningful sense of the word, since as you say, the memory is released immediately afterwards anyway. This is a valid technique for programs without significant lifetime and memory usage. The "leak" detection in valgrind&co. can presumably be switched off. Trying to avoid these so-called leaks significantly complicates what should be a very simple program by requiring every error to be threaded back to the end of the main function. This is error-prone, obscures the important bits of the program, and hinders development. And it's completely pointless. > If you want to > consolidate error handling, which is probably a good idea, then a better > approach is to register each malloc'ed pointer with a cleanup function > [*] and always call that before exit(). > > [...] > > I realize the problem is harder in your version, because many of the > static buf[] variables are now dynamically allocated, but I still think > it's good practice to clean up own messes, even just before calling exit(). Your package, your rules. But my spare time, and I'm not prepared to spend it on this. Oh, and my fault for not asking before making these changes. Feel free to pick the useful bits of code out of the patch. Obviously I'm happy for it to be licensed under GPL2 (or later). Copyright statement not required. Andy
