Merge pull request #498 from SeeSpotRun/backports

Backports

.travis.yml

@@ -4,8 +4,8 @@ install:
     - sudo apt-get update
     - sudo apt-get install scons python3-sphinx gettext python3-setuptools
     - sudo apt-get install libblkid-dev libelf-dev libglib2.0-dev libjson-glib-dev
-    - sudo apt-get install clang
-    - sudo easy_install3 $(cat test-requirements.txt)
+    - sudo apt-get install clang python3-pip python3-cffi libffi-dev
+    - sudo pip3 install -r test-requirements.txt
 
 compiler:
   - clang

SConstruct

@@ -264,29 +264,6 @@ def check_posix_fadvise(context):
     return rc
 
 
-def check_faccessat(context):
-    # Seems to be missing in Mac OSX <= 10.9
-    rc = 1
-
-    if tests.CheckDeclaration(
-        context, 'faccessat',
-        includes='#include <unistd.h>'
-    ):
-        rc = 0
-
-    if rc == 1 and tests.CheckDeclaration(
-        context, 'AT_FDCWD',
-        includes='#include <fcntl.h>'
-    ):
-        rc = 0
-
-    conf.env['HAVE_FACCESSAT'] = rc
-
-    context.did_show_result = True
-    context.Result(rc)
-    return rc
-
-
 def check_xattr(context):
     rc = 1
 
@@ -590,7 +567,6 @@ conf = Configure(env, custom_tests={
     'check_sha512': check_sha512,
     'check_blkid': check_blkid,
     'check_posix_fadvise': check_posix_fadvise,
-    'check_faccessat': check_faccessat,
     'check_sys_block': check_sys_block,
     'check_bigfiles': check_bigfiles,
     'check_c11': check_c11,
@@ -726,7 +702,6 @@ conf.check_sha512()
 conf.check_gettext()
 conf.check_linux_limits()
 conf.check_posix_fadvise()
-conf.check_faccessat()
 conf.check_btrfs_h()
 conf.check_linux_fs_h()
 conf.check_uname()

lib/SConscript

@@ -37,7 +37,6 @@ def build_config_template(target, source, env):
             HAVE_BTRFS_H=env['HAVE_BTRFS_H'],
             HAVE_MM_CRC32_U64=env['HAVE_MM_CRC32_U64'],
             HAVE_BUILTIN_CPU_SUPPORTS=env['HAVE_BUILTIN_CPU_SUPPORTS'],
-            HAVE_FACCESSAT=env['HAVE_FACCESSAT'],
             HAVE_UNAME=env['HAVE_UNAME'],
             HAVE_SYSMACROS_H=env['HAVE_SYSMACROS_H'],
             VERSION_MAJOR=VERSION_MAJOR,

lib/cfg.c

@@ -106,18 +106,28 @@ void rm_cfg_set_default(RmCfg *cfg) {
 }
 
 guint rm_cfg_add_path(RmCfg *cfg, bool is_prefd, const char *path) {
-    int rc = 0;
-
-#if HAVE_FACCESSAT
-    rc = faccessat(AT_FDCWD, path, R_OK, AT_EACCESS|AT_SYMLINK_NOFOLLOW);
-#else
-    rc = access(path, R_OK);
-#endif
+    int rc = access(path, R_OK);
 
     if(rc != 0) {
-        rm_log_warning_line(_("Can't open directory or file \"%s\": %s"), path,
-                            strerror(errno));
-        return 0;
+        /* We have to check here if it's maybe a symbolic link.
+         * Do this by checking with readlink() - if it succeeds
+         * it is most likely a symbolic link. We do not really need
+         * the link path, so we just a size-one array.
+         *
+         * faccessat() cannot be trusted, since it works differently
+         * on different platforms (i.e. between glibc and musl)
+         * (lesson learned, see https://github.com/sahib/rmlint/pull/444)
+         * */
+        char dummy[1] = {0};
+        rc = readlink(path, dummy, 1);
+        if(rc < 0) {
+            rm_log_warning_line(
+                _("Can't open directory or file \"%s\": %s"),
+                path,
+                strerror(errno)
+            );
+            return 0;
+        }
     }
 
     bool realpath_worked = true;

lib/config.h.in

@@ -20,7 +20,6 @@
 #define HAVE_POSIX_FADVISE ({HAVE_POSIX_FADVISE})
 #define HAVE_BTRFS_H       ({HAVE_BTRFS_H})
 #define HAVE_LINUX_FS_H    ({HAVE_LINUX_FS_H})
-#define HAVE_FACCESSAT     ({HAVE_FACCESSAT})
 #define HAVE_UNAME         ({HAVE_UNAME})
 #define HAVE_SYSMACROS_H   ({HAVE_SYSMACROS_H})
 #define HAVE_MM_CRC32_U64  ({HAVE_MM_CRC32_U64})

lib/formats/csv.c

@@ -75,11 +75,11 @@ static void rm_fmt_elem(_UNUSED RmSession *session, _UNUSED RmFmtHandler *parent
 
     if(file->digest != NULL) {
         checksum_size = rm_digest_get_bytes(file->digest) * 2 + 1;
-        checksum_str = g_slice_alloc0(checksum_size);
+        checksum_str = g_malloc0(checksum_size);
         checksum_str[checksum_size - 1] = 0;
         rm_digest_hexstring(file->digest, checksum_str);
     } else {
-        checksum_str = g_slice_alloc0(1);
+        checksum_str = g_malloc0(1);
         *checksum_str = 0;
     }
 
@@ -99,7 +99,7 @@ static void rm_fmt_elem(_UNUSED RmSession *session, _UNUSED RmFmtHandler *parent
     g_free(clean_path);
 
     if(checksum_str != NULL) {
-        g_slice_free1(checksum_size, checksum_str);
+        g_free(checksum_str);
     }
 }
 

tests/test_mains/test_dedupe.py

@@ -6,6 +6,7 @@
 from nose.plugins.skip import SkipTest
 from contextlib import contextmanager
 import psutil
+import re
 
 from tests.utils import *
 
@@ -177,7 +178,7 @@ def pattern_count(path, patterns):
     with open(path, 'r') as f:
         for line in f:
             for i, pattern in enumerate(patterns):
-                if line.startswith(pattern):
+                if re.match(pattern, line):
                     counts[i] += 1
     return counts
 
@@ -201,7 +202,7 @@ def test_clone_handler():
         )
 
     # parse output file for expected clone command
-    counts = pattern_count(sh_path, ["clone '", "skip_reflink '"])
+    counts = pattern_count(sh_path, ["^clone *'", "^skip_reflink *'"])
     assert counts[0] == 1
     assert counts[1] == 0
 
@@ -221,6 +222,6 @@ def test_clone_handler():
             with_json=False
         )
 
-    counts = pattern_count(sh_path, ["clone '", "skip_reflink '"])
+    counts = pattern_count(sh_path, ["^clone *'", "^skip_reflink *'"])
     assert counts[0] == 0
     assert counts[1] == 1

tests/test_robustness/test_badlinks_as_args.py

@@ -0,0 +1,33 @@
+#!/usr/bin/env python3
+# encoding: utf-8
+from nose import with_setup
+from tests.utils import *
+
+
+# Regression test for directly passing broken symbolic links
+# to the command line. See https://github.com/sahib/rmlint/pull/444
+@with_setup(usual_setup_func, usual_teardown_func)
+def test_bad_symlinks_as_direct_args():
+    create_file('xxx', 'a')
+    create_file('xxx', 'b')
+
+    # Create symbolic links:
+    create_link('a', 'link_a', symlink=True)
+    create_link('b', 'link_b', symlink=True)
+
+    link_a_path = os.path.join(TESTDIR_NAME, 'link_a')
+    link_b_path = os.path.join(TESTDIR_NAME, 'link_b')
+
+    # Remove original files:
+    os.remove(os.path.join(TESTDIR_NAME, 'a'))
+    os.remove(os.path.join(TESTDIR_NAME, 'b'))
+
+    # Directly point rmlint to symlinks, should result
+    # in directly finding them.
+    head, *data, footer = run_rmlint(link_a_path, link_b_path)
+    assert len(data) == 2
+    assert data[0]['type'] == 'badlink'
+    assert data[1]['type'] == 'badlink'
+
+    assert {data[0]['path'], data[1]['path']} == \
+            {link_a_path, link_b_path}

tests/utils.py

@@ -18,6 +18,7 @@
 
 
 TESTDIR_NAME = os.getenv('RM_TS_DIR') or '/tmp/rmlint-unit-testdir'
+TESTDIR_NAME = os.path.realpath(TESTDIR_NAME)
 
 CKSUM_TYPES = [
     'murmur',