Hi Loïc,
On Wednesday 28 January 2009, Loïc Minier wrote:
> I was reading the qcontrol source and found a couple oddities which
> are probably harmless but could as well be fixed.
I've forwarded your comments to the upstream author. Thanks.
> While I'm at it, I also spotted this grep|head| sed constructs in 4
> places:
Yes, it's not the most efficient, but has the advantage of readability.
And it's not really performance-critical here.
> device=$(grep "Hardware[[:space:]]*:" /proc/cpuinfo 2>/dev/null | \
> head -n1 | sed "s/^[^:]*: //")
>
> You might want to use something like:
> device=$(sed -rn 's/^Hardware[[:space:]]+: //p' /proc/cpuinfo | head -1)
head -1 won't work in D-I; would have to be -n1.
> for readability or the following pure sed implementation:
Well, that already requires a bit more than basic sed knowledge.
> device=$(sed -rn '/^Hardware[[:space:]]+:/ { s/[^:]+: //p; q }' /proc/cpuinfo)
Nice, but does IMO require a comment to explain what it does.
> albeit I'm not sure why you need 2>/dev/null.
There's always the slight risk that /proc is not mounted:
$ sed -rn '/^modela[[:space:]]+:/ { s/[^:]+: //p; q }' /proc/cpuinfo
sed: can't read /proc/cpuinfo: No such file or directory
Having said all that, I will consider streamlining this a bit for my next
upload (and maybe do the same in D-I).
Cheers,
FJP
--
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]