[Buildroot] [PATCH 3/4 v4] pkg-download: verify the hashes from the download wrapper

Thomas Petazzoni thomas.petazzoni at free-electrons.com
Thu Dec 11 20:42:09 UTC 2014


Dear Yann E. MORIN,

On Thu, 11 Dec 2014 19:24:47 +0100, Yann E. MORIN wrote:

> diff --git a/support/download/check-hash b/support/download/check-hash
> index 13e361a..b41a87e 100755
> --- a/support/download/check-hash
> +++ b/support/download/check-hash
> @@ -5,9 +5,11 @@ set -e
>  # Call it with:
>  #   $1: the path of the file containing all the the expected hashes
>  #   $2: the full path to the file to check
> +#   $3: the basename of the file to check
>  
>  h_file="${1}"
>  file="${2}"
> +base="${3}"

I was very confused here by $2 and $3. If you have "the full path to
the file to check", why would you need "the basename of the file to
check" as argument. I believe this should be clarified:

 $2: the full path to the temporary file that was downloaded, and that
     should be checked

 $3: the name of the real file we are downloading. Used only for
     messages, as we are in fact checking a temporary file, and waiting
     for this temporary file to be fully downloaded and checked before
     renaming it to the real file name.

Of course, feel free to reword this in a better way.

> -            if [ "${f}" = "${file##*/}" ]; then
> +            if [ "${f}" = "${base}" ]; then

Hum, so it is not only used for display. So you need to reword the
above.

>      # tmp_output is in the same directory as the final output, so we can
>      # later move it atomically.
>      tmp_output="$( mktemp "${output}.XXXXXX" )"
> @@ -147,7 +159,7 @@ DESCRIPTION
>              bzr     Bazaar
>              cp      local files via simple copy
>              cvs     Concurrent Versions System
> -            git     git
> +            git     Git

Should be part of a previous patch.

>              hg      Mercurial
>              scp     remote files via Secure copy

Maybe you want to capitalize "Local files..." and "Remote files..." as
well, for consistency.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


More information about the buildroot mailing list