[syslinux] [PATCH] Refactor checksize.pl

[Celelibi]

2015-11-19 17:30 UTC+01:00, Nicolas Cornu via Syslinux <syslinux at zytor.com>:
> - Remove padsize argument as it is never used.
> - Add an usage printed when $file is not set or --help, -h
>   is the first argument.
> - Add basic tests for this script.
> ---
>  mbr/checksize.pl      | 27 ++++++++++++++++++---------
>  mbr/checksize_test.sh | 22 ++++++++++++++++++++++
>  2 files changed, 40 insertions(+), 9 deletions(-)
>  create mode 100644 mbr/checksize_test.sh
>
> diff --git a/mbr/checksize.pl b/mbr/checksize.pl
> index 4b42327..cbca5b2 100755
> --- a/mbr/checksize.pl
> +++ b/mbr/checksize.pl
> @@ -19,25 +19,34 @@
>
>  use bytes;
>
> -($file, $maxsize, $padsize) = @ARGV;
> +($file, $maxsize) = @ARGV;
> +
> +if (!defined($file) or $file == "--help" or $file == "-h") {
> +    $usage = <<"END_USAGE";
> +Usage: $0 file [maxsize]
> +If maxsize is not given it will be guessed based on filename:
> +  - 440 bytes for mbr and gptmbr;
> +  - 432 bytes for isohdp;
> +  - 439 bytes for altmbr.
> +END_USAGE
> +    die $usage;
> +}
>
>  if (!defined($maxsize)) {
>      # Defaults based on the filename
>      if ($file =~ /^mbr[^0-9a-z]/) {
> -	$maxsize = $padsize = 440;
> +	$maxsize = 440;
>      } elsif ($file =~ /^gptmbr[^0-9a-z]/) {
> -	$maxsize = $padsize = 440;
> +	$maxsize = 440;
>      } elsif ($file =~ /^isohdp[fp]x[^0-9a-z]/) {
> -	$maxsize = $padsize = 432;
> +	$maxsize = 432;
>      } elsif ($file =~ /^altmbr[^0-9a-z]/) {
> -	$maxsize = $padsize = 439;
> +	$maxsize = 439;
>      } else {
>  	die "$0: no default size for filename: $file\n";
>      }
>  }
>
> -$padsize = $maxsize unless(defined($padsize));
> -
>  open(FILE, '+<', $file) or die;
>  @st = stat(FILE);
>  if (!defined($size = $st[7])) {
> @@ -46,9 +55,9 @@ if (!defined($size = $st[7])) {
>  if ($size > $maxsize) {
>      print STDERR "$file: too big ($size > $maxsize)\n";
>      exit 1;
> -} elsif ($size < $padsize) {
> +} elsif ($size < $maxsize) {
>      seek(FILE, $size, 0);
> -    print FILE "\0" x ($padsize-$size);
> +    print FILE "\0" x ($maxsize-$size);
>  }
>
>  exit 0;
> diff --git a/mbr/checksize_test.sh b/mbr/checksize_test.sh
> new file mode 100644
> index 0000000..d315118
> --- /dev/null
> +++ b/mbr/checksize_test.sh
> @@ -0,0 +1,22 @@
> +#!/bin/sh
> +
> +mkdir -p output_test && cd output_test || exit 1
> +rm -f a mbr.bin
> +touch a
> +# Upgrade size of a file should success
> +perl ../checksize.pl a 330
> +[ $(wc -c a | cut -f1 -d" ") -eq 330 ] || exit 2
> +# Downgrade size of a file should fail
> +perl ../checksize.pl a 300
> +[ $? -eq 1 ] || exit 3
> +rm a
> +
> +touch mbr.bin
> +# Upgrade size of a file
> +perl ../checksize.pl mbr.bin 330
> +[ $(wc -c mbr.bin | cut -f1 -d" ") -eq 330 ] || exit 4
> +# Let's upgrade it by itself
> +perl ../checksize.pl mbr.bin
> +[ $(wc -c mbr.bin | cut -f1 -d" ") -eq 440 ] || exit 5
> +rm mbr.bin
> +
> --
> 2.6.2

Just to make sure: the goal of the shell script is to check the perl script?
If so, I wonder if it wouldn't be better in the test directory if it
is needed at all. I'll let the others comment on that.

I agree with your simplification of checksize.pl, but have you
searched through the history to see if there wasn't a good reason to
separate maxsize and padsize?


Celelibi