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