----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34138/#review90791 -----------------------------------------------------------
1. Agree that this is useful as a utility in libprocess. Not much overhead to move it over right? 2. It feels like something that could be exposed as a function rather than class, maybe a TODO. src/slave/containerizer/provisioners/appc/hash.hpp (lines 22 - 31) <https://reviews.apache.org/r/34138/#comment143922> ``` #include <string> #include <vector> #include <stout/...> #include <process/...> ... ``` According to the style guide. src/slave/containerizer/provisioners/appc/hash.hpp (lines 45 - 55) <https://reviews.apache.org/r/34138/#comment143917> I would do ``` if (!system("sha512sum -h &> /dev/null")) {...} ``` to avoid fixing the binary location. src/slave/containerizer/provisioners/appc/hash.hpp (lines 89 - 91) <https://reviews.apache.org/r/34138/#comment143948> I think we use 4 spaces to continue a left angle bracket the same way we continue an left paren. src/slave/containerizer/provisioners/appc/hash.hpp (line 97) <https://reviews.apache.org/r/34138/#comment143939> The `command` is not necessarily `sha512sum`. Maybe use it a static member so we detect once and use it with every hash() invocation? src/slave/containerizer/provisioners/appc/hash.hpp (line 98) <https://reviews.apache.org/r/34138/#comment143940> Misaligned indentation. src/slave/containerizer/provisioners/appc/hash.hpp (line 102) <https://reviews.apache.org/r/34138/#comment143949> The `command` is not necessarily `sha512sum`. src/slave/containerizer/provisioners/appc/hash.hpp (line 109) <https://reviews.apache.org/r/34138/#comment143941> Misaligned indentation. src/slave/containerizer/provisioners/appc/hash.hpp (lines 112 - 122) <https://reviews.apache.org/r/34138/#comment143946> This check doesn't work with openssl. ``` /usr/bin/openssl dgst -sha512 somefile.txt SHA512(somefile.txt)= 5a73e55fd845981be5d5b87039c678b87404405d5d054c579cf684a18893d181085b9afde535c034221f858d2bcc2b14978b4d5f4d6facfaa1f81e727a010f3c ``` I think we only need shasum and sha512sum to cover both Linux and OSX. - Jiang Yan Xu On July 7, 2015, 12:42 p.m., Ian Downes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34138/ > ----------------------------------------------------------- > > (Updated July 7, 2015, 12:42 p.m.) > > > Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone. > > > Repository: mesos > > > Description > ------- > > AppC hash computation. > > > Diffs > ----- > > src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 > src/slave/containerizer/provisioners/appc/hash.hpp PRE-CREATION > > Diff: https://reviews.apache.org/r/34138/diff/ > > > Testing > ------- > > > Thanks, > > Ian Downes > >
