-1

So I have a script that iterates through all the files in a directory and checks whether the files contain the same content or not with any other files in the same directory:

Check() 
{
    if [ -e "new.txt" ]
    then
        rm new.txt
    fi
    for item in "$2"/*
    do
        if [ ! -d "$item" ]
        then
            diff "$1" "$item">new.txt
            if [ -s "new.txt" ]
            then
                echo "$1 $item"
                echo "Yes"
            fi
        fi
    done
}

Iterate()
{
    for item in "$2"/*
    do
        if [ ! -d "$item" ]
        then
            Check $item $2
        fi
    done
}

Iterate $1 $2

And the bash

bash script.sh asd /home/ljuben

However when I run the script it always echoes "Yes" eve if the files don't contain the same content.

And ideas?

  • 2
    @Jesse_b I don't think that's quite true; unless the -s option is given, diff will produce an empty file if the files are the same: however [ -s "new.txt" ] will return true if new.txt exists and is not empty (inverting the OP's intended logic) – steeldriver Jun 15 '18 at 15:34
  • Regardless, as @Jesse_b and @Kusalananda have pointed out, it would be simpler and better practice to check the exit status of your comparison command rather than creating and testing a file: e.g. if ! cmp -s "$1" "$item"; then echo "Different"; fi – steeldriver Jun 15 '18 at 15:51
  • What is asd on your command line and why don't you use that argument in your script? – Kusalananda Jun 15 '18 at 16:18
  • It's nothing. I started it because I thought it must be made with two arguments but then I realized that I can make it with one – David Mathers Jun 15 '18 at 16:22
2

Your script seems to not use its first argument. It is passed to the Iterate function, and then never seen again.

But the real issue is that you run diff on every combination of two files, and then look at the size of the diff. The size of the diff will be non-zero for files that are different. Your script is therefore reporting Yes for every combination of files that are different, not the same.

You also needlessly run the diff between file A and B twice (A vs. B and then later B vs. A). You can get by this by generating the file list only once, and then iterating over that.

Alternative script:

#!/bin/sh

if [ ! -d "$1" ]; then
    printf 'Usage: %s directory\n' "$0" >&2
    exit 1
fi

# set positional parameters to the list of files in the given directory
set -- "$1"/*

# while there's still files to process...
while [ "$#" -gt 0 ]; do
    # skip over non-regular files
    if [ ! -f "$1" ]; then
        shift
        continue
    fi

    # now process the first item in the list against the other ones
    item=$1

    # shift off the first item from the list
    shift

    # loop over the remaining items...
    for name do
        # we're still not interested in non-regular files
        [ ! -f "$name" ] && continue

        # if they are the same, report this
        if cmp -s "$item" "$name"; then
            printf '%s and %s has same content\n' "$item" "$name"
        fi
    done
done

You may still have your two functions if you wish:

#!/bin/sh

if [ ! -d "$1" ]; then
    printf 'Usage: %s directory\n' "$0" >&2
    exit 1
fi

check () {
    # now process the first item in the list against the other ones
    item=$1

    # shift off the first item from the list
    shift

    # loop over the remaining items...
    for name do
        # we're still not interested in non-regular files
        [ ! -f "$name" ] && continue

        # if they are the same, report this
        if cmp -s "$item" "$name"; then
            printf '%s and %s has same content\n' "$item" "$name"
        fi
    done
}

iterate () {
    # set positional parameters to the list of files in the given directory
    set -- "$1"/*

    # while there's still files to process...
    while [ "$#" -gt 0 ]; do
        # only process regular files
        if [ -f "$1" ]; then
            check "$@" # checks the first item against the rest
        fi
        shift # get rid of the first item
    done
}

iterate "$1"

Notice how we don't let the check function generate its own file list. Instead we pass the list of files to it.

For the lazy:

fdupes -1 /some/directory
0

You want to use diff with the -s option:

-s, --report-identical-files

report when two files are the same

You also don't need to create a file with the output, you can just test on the exit of the diff command:

Check() 
{
    if [ -e "new.txt" ]
    then
        rm new.txt
    fi
    for item in "$2"/*
    do
        if [ ! -d "$item" ]
        then 
            if diff -s "$1" "$item"
            then
                echo "$1 $item"
                echo "Yes"
            fi
        fi
    done
}

As Kusalananda pointed out cmp is probably a better option and is more portable. You could use:

if cmp -s "$1" "$item"
then
    echo "$1 $item"
    echo "Yes"
fi
  • 2
    See also cmp -s which is more portable. – Kusalananda Jun 15 '18 at 15:23
  • Nope it still doesn't work – David Mathers Jun 15 '18 at 15:40
  • 1
    "Still doesn't work" doesn't help much. What about it doesn't work? – Jesse_b Jun 15 '18 at 15:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.