Review Request 110091: clean up and update FreeBSD support for kinfocenter

Rolf Eike Beer kde at opensource.sf-tec.de
Thu May 2 15:44:43 BST 2013


Am 02.05.2013 15:12, schrieb Max Brazhnikov:
> On Wed, 01 May 2013 14:44:39 +0200 Rolf Eike Beer wrote:
>>>> On April 20, 2013, 2:11 p.m., Rolf Eike Beer wrote:
>>>>> kinfocenter/Modules/base/info_fbsd.cpp, line 136
>>>>> <http://git.reviewboard.kde.org/r/110091/diff/1/?file=139992#file139992l
>>>>> ine136>> >
>>>>>     Why not just use QProcess here to get the result? I fear this 
>>>>> stuff
>>>>>     dates back to QT(<=3) times where this probably had issues, 
>>>>> but
>>>>>     that isn't true anymore.
>>> GetInfo_ReadfromPipe already uses QProcess.
>> 
>> Hm, ok. But the 2>&1 will not work, as that is shell syntax for 
>> redirects and
>> I'm not sure if QProcess will unterstand that. What I basically had 
>> in mind
>> was to pass a QStringList to the functions so that the arguments are 
>> already
>> properly separated. Basically what the commenter here had in mind
>> (info_fbsd.cpp):
>> 
>> // TODO: GetInfo_ReadfromPipe should be improved so that we could 
>> pass the
>> program name and its
>> //       arguments to it and remove most of the code below.
>> 
>> So it would be nice if you could clean up that, too.
> 
> Ok, here's a patch for GetInfo_ReadfromPipe only:
> http://people.freebsd.org/~makc/patches/read_from_pipe.diff

Looks good to me (although I have not tested it). Small note: there is 
a contructor of QStringList taking a QString so if you have a list with 
only one item you don't need to construct an empty list and then add 
something using operator<<.

Side-note: there is a comment talking about using /proc/pci if lspci is 
not found. Not that newer kernels do have a /proc/pci at all ;)

>> While you're at it you
>> can then fix the type (devies -> devices), too ;)
> 
> I've removed those comments completely.

Another solution for that problem ;)

Eike




More information about the kde-core-devel mailing list