Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backports #498

Merged
merged 5 commits into from
May 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 0 additions & 25 deletions SConstruct
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down
1 change: 0 additions & 1 deletion lib/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
30 changes: 20 additions & 10 deletions lib/cfg.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion lib/config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down
6 changes: 3 additions & 3 deletions lib/formats/csv.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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);
}
}

Expand Down
7 changes: 4 additions & 3 deletions tests/test_mains/test_dedupe.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from nose.plugins.skip import SkipTest
from contextlib import contextmanager
import psutil
import re

from tests.utils import *

Expand Down Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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
33 changes: 33 additions & 0 deletions tests/test_robustness/test_badlinks_as_args.py
Original file line number Diff line number Diff line change
@@ -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}
1 change: 1 addition & 0 deletions tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down