[procps] Re: [PATCH 1/2] top: restore terminal state on exit
- From: Jim Warner <james.warner@xxxxxxxxxxx>
- To: procps@xxxxxxxxxxxxx
- Date: Tue, 1 May 2012 03:01:49 -0500
On Apr 30, 2012, at 12:33 PM, Mike Frysinger wrote:
> as i said in a private e-mail:
> i still think the atexit hook should be added regardless of any other /proc
> testing top acquires. assuming the code will always handle all edge cases
> all
> the time and will never unexpectedly exit is naïve.
>
> also, i'd point out that "amount of code threw at the problem" is a bit
> melodramatic. i added 1 new function call, and split out code that already
> existed in the bye_bye() function into a dedicated restore_terminal()
> function.
Mike,
Here are the reasons I objected to your patches...
1) While probably not a factor with /proc, I prefer to not use a function like
your access() whose own documentation suggests "use of this system call should
be avoided".
2) You disrespected my main function with unnecessay bloat. I've tried to keep
main() as short as possible, concentrating on the run-time loop. Low level
start up details were encapsulated in other functions. And while I tolerated
Sami's recent atexit() addition, the code you added more properly belongs in
the before() routine.
3) You violated the alphabetical order of functions within sections when you
added restore_terminal(). And while that doesn't impact the executable in the
least, I consider the current discipline a useful programmer challenge.
commit 270e8e7eeb66b47569940f537cdad4ab46cd36be
4) You failed to update the pseudo prototypes that have been carefully
maintained in top.h. I don't know if anyone else has ever used that
documentation for a quick program overview or source code navigation, but even
as the program author I have.
commit efb9fcb81604941a28532f9df841eb93bde7f653
commit 270e8e7eeb66b47569940f537cdad4ab46cd36be
5) You completely ignored nls support with your new message. The existing
library message emitted under my approach also bypasses nls but, unlike your
message, there is precious little to translate with the library text.
commit e64b6f70d7c99db01d7270c27f09de5b02cacebb
commit db88a8d2420664ab108d6aa5400f2cea6ac660d5
Regards,
Jim
Other related posts:
