On 2010-Feb-16 09:59:46 -0800, Jeremy Chadwick <free...@jdc.parodius.com> wrote: >On Tue, Feb 16, 2010 at 11:06:43AM -0600, Barry Pederson wrote: >> maybe something like this tacked on the end of the script (excuse my >> Perl, I'm a Python guy). >> >> ---- >> #### Loader Settings ############# >> open(LOADER, '/boot/loader.conf'); >> print "\n/boot/loader.conf settings:\n"; >> while (<LOADER>){ >> chomp; >> if (/^\s*(zfs|vfs\.zfs|vm\.kmem)/){ >> print "\t$_\n"; >> } >> } >> ---- > >Major problems with the above code: > >1) Opens /boot/loader.conf for rw access; should be read-only Wrong. It opens /boot/loader.conf read-only, as it should.
>2) Makes the assumption /boot/loader.conf exists Accepted but it was offered as a proof-of-concept. >3) Does not close the fd This is hardly a "major problem" in a short once-through script. >4) Excessively quotes variables for no justified reason If you mean including the $_ inside "", this is a standard perl idiom. >5) Makes some bad assumptions about the contents of the file (ex. > comments with the word "zfs" in them would match) Wrong. It only matches "zfs" as the first non-blank text on a line - which means it can't be a comment. So that's one real deficiency in a proof-of-concept script written by someone who does not claim to be comfortable with perl. >The code should really be something like what's below. This should >be much more manageable as well (@tunables that is), although I always >worry when using grep()... Whilst we are picking nits, your script has the following issues: - unnecessary trailing wildcard matches in the regexps - "grep" misused as "map" - "die" is probably not appropriate for embedding into another script - No useful error message if /boot/loader.conf can't be opened. - Doesn't correctly handle optional whitespace around "=" - No heading to explain what is being reported. - Doesn't allow for "zfs" as a top-level identifier Overall, Barry's script makes an excellent proof-of-concept - which is what he was offering. -- Peter Jeremy
pgpI6dXwSO06C.pgp
Description: PGP signature