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 debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to