Li, Aubrey wrote: > Hi Rafael, > > A nice work, especially the work for window resizing!
Thanks :) > Rafael.Vanoni <> wrote: > >> Just posted a webrev at >> http://cr.opensolaris.org/~rafaelv/ptop-display/ with suggested fixes >> for the following bugs >> >> 11928 powertop(1) doesn't refresh data if system only supports C0 >> 11952 PowerTOP's window resizing doesn't always refresh the screen >> 11863 i18n and pt_error() (remove locale.h) >> >> Please see each bug report for a description of the changes and >> individual patches, and let me know what you think. >> >> Thanks, >> Rafael > > several comments below: > > 1) this changeset caused compiling warning on my side. Did you see it? > > "../common/powertop.c", line 115: warning: declaration can not follow > a statement (E_DECLARATION_IN_CODE > - - - - - - - - - - - - - - - - - - - - - - - - > boolean_t root_user = B_FALSE; > + struct pollfd pollset; > > + pollset.fd = STDIN_FILENO; > + pollset.events = POLLIN; > + > static struct option opts[] = { > { "dump", 1, NULL, 'd' }, > { "time", 1, NULL, 't' }, > - - - - - - - - - - - - - - - - - - - - - - - - > of course we should put pollset initialization after struct option definition. It works for me on ptop-core, haven't tried it on ptop-full. Maybe it's a path or missing library problem (?) > 2) display.c > line 117, why putchar('r') is necessary here? If you resize the window to a shorter height and quit, the terminal line will sometimes 'garbled' by output. This just clears it (I got the idea from prstat(1)). > 3) powertop.c > line240~249, is it better to put pollset initlization to here? Ok. > line271~275, for what reason poll is used here, instead select? There is some signal interaction between select(3) and catching the SIGWINCH signal that was causing the window not to be refreshed after a resize. Using poll(3) fixed it. Thanks, Rafael
