From kde-core-devel Sun Nov 10 00:43:50 2013 From: "David Stephen Hubner" Date: Sun, 10 Nov 2013 00:43:50 +0000 To: kde-core-devel Subject: Re: Review Request 113779: KInfocenter/OpenGL: fix ReadPipe() in the case that the command cannot be Message-Id: <20131110004350.31140.23501 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=kde-core-devel&m=138404425804332 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============1048154623877803458==" --===============1048154623877803458== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113779/#review43314 ----------------------------------------------------------- Ship it! Looks good to me - David Stephen Hubner On Nov. 10, 2013, 12:20 a.m., Wolfgang Bauer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/113779/ > ----------------------------------------------------------- > > (Updated Nov. 10, 2013, 12:20 a.m.) > > > Review request for kde-workspace and David Stephen Hubner. > > > Bugs: 327382 > http://bugs.kde.org/show_bug.cgi?id=327382 > > > Repository: kde-workspace > > > Description > ------- > > ReadPipe() doesn't return 0 as expected in the case that the command is not found. but the length of sh's output which is "command not found" in this case. This is because popen() does not fail if the command is not found, because it _can_ run "sh". (according to the man page, popen calls "/bin/sh -c command") > To fix this, ReadPipe() should check the return code of the call to pclose() (see "man pclose"), and return 0 if this is not equal to 0. > > > Diffs > ----- > > kinfocenter/Modules/opengl/opengl.cpp 7df2b17 > > Diff: http://git.reviewboard.kde.org/r/113779/diff/ > > > Testing > ------- > > Run KInfocenter on openSUSE, where lspci is in /sbin and that is not in the user's path. > Without this patch, 3D Accelerator will be shown as "unknown" (because lspci cannot be run, with this patch it works as intended. > I also tried with lspci in /usr/bin/ (i.e. in the path) and completely removed, worked as expected (correct information in the former case, "unknown" in the latter). > > > Thanks, > > Wolfgang Bauer > > --===============1048154623877803458== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113779/

Ship it!

Looks good to me

- David Stephen Hubner

On November 10th, 2013, 12:20 a.m. UTC, Wolfgang Bauer wrote:

Review request for kde-workspace and David Stephen Hubner.

By Wolfgang Bauer.

Updated Nov. 10, 2013, 12:20 a.m.

Bugs: 327382

Repository: kde-workspace

Description

ReadPipe() doesn't return 0 as expected in the case that the command is not found. but the length of sh's output which is "command not found" in this case. This is because popen() does not fail if the command is not found, because it _can_ run "sh". (according to the man page, popen calls "/bin/sh -c command")
To fix this, ReadPipe() should check the return code of the call to pclose() (see "man pclose"), and return 0 if this is not equal to 0.

Testing

Run KInfocenter on openSUSE, where lspci is in /sbin and that is not in the user's path.
Without this patch, 3D Accelerator will be shown as "unknown" (because lspci cannot be run, with this patch it works as intended.
I also tried with lspci in /usr/bin/ (i.e. in the path) and completely removed, worked as expected (correct information in the former case, "unknown" in the latter).

Diffs

kinfocenter/Modules/opengl/opengl.cpp (7df2b17)

--===============1048154623877803458==--