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

[Igor Zhbanov <i.zhbanov at samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 3353 bytes --]

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