[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.


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


Other related posts: