You ask for input, so, here goes
- You are using an indentation of 8 characters. This is not just a style issue, it actually makes the code harder to apprehend rather than using 2, 3 or 4 character indents. I believe the seminal reference on the matter is Steve Connell's "Code Complete". In fact the indentation is a bit borked in places, you should run it through Perltidy or similar pretty printer to get things into line.
- The variables $b and $i appear to be used only to count interations, and the iterations are governed by a C-style for loop. A more perlish idiom would be something like
my $iter = 0;
while( $iter++ < $MAX_ITERS ) {
do_stuff();
}
In any event, you have to get rid of the globals. Many of them are needed only for nuke_file(), so define them there, or wrap them up in tight scopes like:
{
my $block_size = 207;
sub get_block_size { $block_size }
}
This means that once you leave the scope, you can no longer change the block size, you can only retrieve it. Which is just what you want.
- The main problem with the code is with the sub nuke_file. You are passing in a modal variable that controls how the subroutine is to behave. This is a maintenance nightmare. What does it have to do? Write a stream of bytes equal to the length of that file. What does the stream of bytes contain? Who cares? What if you want to add 01010101 and/or 10101010 as possible bit streams? A different random stream (such as /dev/random or /dev/srandom, i.e. a random stream that blocks if you drain the entropy pool)?
The most open-ended way to deal with this problem is to pass the subroutine a callback. You open the file, and then call the callback for each block of data you want to write (although I would choose a default block size of 4096 bytes rather than 8). A rough sketch would look something like
sub nuke_file {
my $file = shift;
my $cb = shift or die "No callback specified to nuke_file().\n";
die "$cb is not a coderef in nuke_file()\n" unless ref($cb) eq 'CO
+DE';
my $file_size = (stat($file))[7];
my $bytes = 0;
my $block_size = get_block_size();
sysopen(BYEBYE, $file, O_WRONLY)
# be descriptive in your die string
or die "Could not open $file for output: $!\n";
while( $bytes < $file_size ) {
# deal with the tail end of the file correctly
my $len = $bytes + $block_size > $file_size
? $file_size - $bytes
: $block_size;
my $written = syswrite(BYEBYE, &$cb( $len ), $len, 0)
or die "Could not write $len bytes to $file: $!";
my $bytes += $written; # $err is a misleading name
}
close(BYEBYE);
}
- This would then get rid of the ugly hacks $one and $null, which I found to have misleading names. Naming them $all_ones and $all_zeros would be closer to the point. I don't know if I'm alone in the following, but I find $all_ones = chr(255) less clear than simply $all_ones = 0xff, that may be my background, but the use of chr sort of shouts out ASCII! Unportability! to me. And there may be yet a better way.
A sample callback would look like
sub all_ones {
0xff x $_[0];
}
nuke_file( $filename, \&all_ones );
This approach would also let you gracefully degrade performance if Crypt::Blowfish was not available. And conversely, use any Crypt::* module that you know of that performs block ciphering.
- Use the ||= defaulting operator, such as in
my $iterations = $opts{i} || 7
- In a raft of assignments, use whitespace to align the = characters. It takes more time, and is a bear to fix should you add a longer variable name to the batch, but it improves the readability.
- sysopen and sysread are in general rarely needed. Do you need the functionality they provide (bypassing buffering, no partial read lengths spring to mind) or will open and read do? If you can, the program will be a better player in its environment.
- In sub do_help all those prints would be better replaced by a simple heredoc. If you're worried about the interpolation of @gmx, you can always use the <<'HEREDOC' form to specify single quote semantics (no interpolation).
- The Orange Book standard recommends, from dim memory, overwriting by all 0s, all 1s, and then 1 to 9 times with a random bit pattern (the emphasis being on 1 to 9).
--g r i n d e r
print@_{sort keys %_},$/if%_=split//,'= & *a?b:e\f/h^h!j+n,o@o;r$s-t%t#u';