On Tue, Mar 22, 2022 at 08:20:15PM +0800, Zorro Lang wrote:
> On Tue, Mar 22, 2022 at 04:22:59PM +1100, Dave Chinner wrote:
> > On Mon, Mar 21, 2022 at 07:03:40PM +0800, Zorro Lang wrote:
> > > The "od" is one of the most fundamental commands in GNU/Linux and
> > > most Unix-like systems. So we nearly always can count on it, don't
> > > need to check if it's installed.
> > >
> > > The "hexdump" isn't such fundamental as "od", some systems don't
> > > install it by default. And as "od" nearly can replace all functions
> > > of "hexdump", so let's use an unified command "od" to do the hexdump
> > > job in fstests cases.
> > >
> > > Signed-off-by: Zorro Lang <zlang@xxxxxxxxxx>
> >
> > Looks good.
> >
> > Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > > ---
> > >
> > > Hi,
> > >
> > > As Dave and Darrick suggested, I did this change, I've tested most of cases,
> > > except f2fs/001 and ceph/002, but I think they're good. And I used "od"
> > > command directly in generic/404 and generic/042 for their special reason.
> >
> > That generic/404 usage is ... strange. Why record md5sums of the
> > encoded hexdump output of the file when you can just run md5sum on
> > the file directly and get the same information? i.e.
> >
> > > diff --git a/tests/generic/404 b/tests/generic/404
> > > index f1e8b0a8..939692eb 100755
> > > --- a/tests/generic/404
> > > +++ b/tests/generic/404
> > > @@ -110,7 +110,7 @@ for (( block=3; block<=500; block++ )); do
> > > # or blocks are in correct order, this commit:
> > > # 2b3864b32403 ("ext4: do not polute the extents cache while shifting extents")
> > > #
> > > - md5=`hexdump -e '16/1 "%_p" "\n"' $testfile | md5sum`
> > > + md5=`od -An -c $testfile | md5sum`
> > > printf "#%d %s\n" "$block" "$md5"
> >
> > Why isn't this just:
> >
> > - md5=`hexdump -e '16/1 "%_p" "\n"' $testfile | md5sum`
> > - printf "#%d %s\n" "$block" "$md5"
> > + echo -n "#$block "
> > + md5sum $testfile | _filter_test_dir
>
> Yes, I thought about that too, but I can't be sure about it, so tried to keep
> the logic of original code. If the original author doesn't have some special
> reason, I'd like to change it as this way (cc roman.penyaev@xxxxxxxxxxxxxxxx).
>
> And for the generic/042:
> - if hexdump -v -e '/1 "%02X "' $file | grep -q "CD"; then
> + if od -An -tx1 -v $file | grep -q "CD"; then
>
> I don't know what that "-v" option is needed (cc Darrick). I thought it might just
> waste the time of grep running ?
Probably. I probably put that in there for debugging purposes and then
frogot to take it out.
--D
> >
> > Seperate patch, perhaps?
>
> Sure, if no one object that, I'll change them in separated patch.
>
> Thanks,
> Zorro
>
> >
> > Cheers,
> >
> > Dave.
> > --
> > Dave Chinner
> > david@xxxxxxxxxxxxx
> >
>
