Re: [Powertop] [RFC][PATCH 2/2] Convert powertop to use new report generator facility

Hi Chris, Please check the new version of the patch set (when it will be approved by mailing list moderator because of its size).

Chris Ferron wrote: > > Couple of things, > > #1 I noticed that the scripts section of Software Settings in need of > Tuning is missing. I see the nice header, but not data. Done. > #2 C0 active table cells are not aligned , and the CPU's are also not > aligned correctly in the Processor Idle state report section. Done. I hope. :-) I have aligned table headers as in original Powertop If you mean something else please tell. > #3 I get some warning compiling, I know PowerTOP is not warning less, but > would like to attempt to prevent adding more. (and fixing exiting ones of > course) > report/report-maker.cpp: In destructor ‘report_maker::~report_maker()’: > report/report-maker.cpp:51:10: warning: deleting object of abstract class > type ‘report_formatter’ which has non-virtual destructor will cause > undefined behaviour [-Wdelete-non-virtual-dtor] > report/report-maker.cpp: In member function ‘void > report_maker::setup_report_formatter()’: > report/report-maker.cpp:110:10: warning: deleting object of abstract class > type ‘report_formatter’ which has non-virtual destructor will cause > undefined behaviour [-Wdelete-non-virtual-dtor] Done. > #4 report[c,h] should probable just go in your new sub/dir (report). I will send this patch later because it will make my patch bigger and that could make the understanding more difficult. It could be a separated patch to easily see changes. > #5 csv report has alignment issues for both the cpu Idle and Freq. Even if > they are machine readable, they need to be easily parsed using open office > or MS excel and look aligned with the http report. CSV has a lots of questions. First of all, what CSV standard to follow? Should it be readable by parsers or by Offices? I have tried to make report maker to be CSV RFC compliant, but as I see Offices doesn't like that CSV. They both prefer to use semicolon as separator, and not to quote strings because of spaces inside of them, and not to quote empty strings. The only reason to quote is when the string contains semicolon inside of it. So I have tuned CSV report maker to produce CSVs to be read by Offices. Both (MS and Open) can read it with default settings. The other question is formatting inside the CSV. In my opinion there should not be any space padding for values inside the CSV. First of all, is is a machine-readable format. Second, MS Office ignores it totally. By the way, as I see, both offices uses following text alignment rules: if the cell consists of text (letters) then it will be shown aligned to the left, and if the cell is a number, then it will be shown aligned to the right. This formatting rules are reasonable, so we could totally remove any padding by space in CSV (and still have pretty table in Offices). What you think?

And if we need report in human readable text format, we could create TXT report formatter.

-- Best regards, Igor Zhbanov, Expert Software Engineer, phone: +7 (495) 797 25 00 ext 3806 e-mail: i.zhbanov(a)samsung.com ASWG, Moscow R&D center, Samsung Electronics 12 Dvintsev street, building 1 127018, Moscow, Russian Federation _______________________________________________ PowerTop mailing list PowerTop(a)lists.01.org https://lists.01.org/mailman/listinfo/powertop