From adf0216d98146f925659e8a5f83a9c653ae78b22 Mon Sep 17 00:00:00 2001 From: Jude Taylor Date: Mon, 15 Aug 2016 10:43:14 -0700 Subject: [PATCH] code review comments --- .../resolve-system-dependencies.cc | 249 +++++++++--------- 1 file changed, 123 insertions(+), 126 deletions(-) diff --git a/src/resolve-system-dependencies/resolve-system-dependencies.cc b/src/resolve-system-dependencies/resolve-system-dependencies.cc index d432781d4..641069c2c 100644 --- a/src/resolve-system-dependencies/resolve-system-dependencies.cc +++ b/src/resolve-system-dependencies/resolve-system-dependencies.cc @@ -11,14 +11,11 @@ using namespace nix; -typedef std::map> SetMap; - static auto cacheDir = Path{}; -Path resolveCacheFile(const Path & lib) { - Path lib2 = Path(lib); - std::replace(lib2.begin(), lib2.end(), '/', '%'); - return cacheDir + "/" + lib2; +Path resolveCacheFile(Path lib) { + std::replace(lib.begin(), lib.end(), '/', '%'); + return cacheDir + "/" + lib; } std::set readCacheFile(const Path & file) { @@ -34,169 +31,169 @@ void writeCacheFile(const Path & file, std::set & deps) { fp.close(); } -std::string find_dylib_name(FILE *obj_file, struct load_command cmd) { - fpos_t pos; - fgetpos(obj_file, &pos); - struct dylib_command dylc; - dylc.cmd = cmd.cmd; - dylc.cmdsize = cmd.cmdsize; - fread(&dylc.dylib, sizeof(struct dylib), 1, obj_file); +std::string findDylibName(FILE *obj_file, struct load_command cmd) { + fpos_t pos; + fgetpos(obj_file, &pos); + struct dylib_command dylc; + dylc.cmd = cmd.cmd; + dylc.cmdsize = cmd.cmdsize; + fread(&dylc.dylib, sizeof(struct dylib), 1, obj_file); - char *dylib_name = (char*)calloc(cmd.cmdsize, sizeof(char)); - fseek(obj_file, - // offset is calculated from the beginning of the load command, which is two - // uint32_t's backwards - dylc.dylib.name.offset - (sizeof(uint32_t) * 2) + pos, - SEEK_SET); - fread(dylib_name, sizeof(char), cmd.cmdsize, obj_file); - fseek(obj_file, pos, SEEK_SET); - return std::string(dylib_name); + char *dylib_name = (char*)calloc(cmd.cmdsize, sizeof(char)); + fseek(obj_file, + // offset is calculated from the beginning of the load command, which is two + // uint32_t's backwards + dylc.dylib.name.offset - (sizeof(uint32_t) * 2) + pos, + SEEK_SET); + fread(dylib_name, sizeof(char), cmd.cmdsize, obj_file); + fseek(obj_file, pos, SEEK_SET); + return std::string(dylib_name); } -bool seek_mach64_blob(FILE *obj_file, enum NXByteOrder end) { - struct fat_header head; - fread(&head, sizeof(struct fat_header), 1, obj_file); - swap_fat_header(&head, end); - for(uint32_t narches = 0; narches < head.nfat_arch; narches++) { - struct fat_arch arch; - fread(&arch, sizeof(struct fat_arch), 1, obj_file); - swap_fat_arch(&arch, 1, end); - if(arch.cputype == CPU_TYPE_X86_64) { - fseek(obj_file, arch.offset, SEEK_SET); - return true; +bool seekMach64Blob(FILE *obj_file, enum NXByteOrder end) { + struct fat_header head; + fread(&head, sizeof(struct fat_header), 1, obj_file); + swap_fat_header(&head, end); + for(uint32_t narches = 0; narches < head.nfat_arch; narches++) { + struct fat_arch arch; + fread(&arch, sizeof(struct fat_arch), 1, obj_file); + swap_fat_arch(&arch, 1, end); + if(arch.cputype == CPU_TYPE_X86_64) { + fseek(obj_file, arch.offset, SEEK_SET); + return true; + } } - } - return false; + return false; } std::set runResolver(const Path & filename) { - FILE *obj_file = fopen(filename.c_str(), "rb"); - uint32_t magic; - fread(&magic, sizeof(uint32_t), 1, obj_file); - fseek(obj_file, 0, SEEK_SET); - enum NXByteOrder endianness; - if(magic == 0xBEBAFECA) { - endianness = NX_BigEndian; - if(!seek_mach64_blob(obj_file, endianness)) { - std::cerr << "Could not find any mach64 blobs in file " << filename << ", continuing..." << std::endl; - return std::set(); + FILE *obj_file = fopen(filename.c_str(), "rb"); + uint32_t magic; + fread(&magic, sizeof(uint32_t), 1, obj_file); + fseek(obj_file, 0, SEEK_SET); + enum NXByteOrder endianness; + if(magic == 0xBEBAFECA) { + endianness = NX_BigEndian; + if(!seekMach64Blob(obj_file, endianness)) { + std::cerr << "Could not find any mach64 blobs in file " << filename << ", continuing..." << std::endl; + return std::set(); + } } - } - struct mach_header_64 header; - fread(&header, sizeof(struct mach_header_64), 1, obj_file); - if(!(header.magic == MH_MAGIC_64 || header.magic == MH_CIGAM_64)) { - std::cerr << "Not a mach-o object file: " << filename << std::endl; - return std::set(); - } - std::set libs; - for(uint32_t i = 0; i < header.ncmds; i++) { - struct load_command cmd; - fread(&cmd.cmd, sizeof(uint32_t), 1, obj_file); - fread(&cmd.cmdsize, sizeof(uint32_t), 1, obj_file); - switch(cmd.cmd) { - case LC_LOAD_DYLIB: - case LC_LOAD_UPWARD_DYLIB: - case LC_REEXPORT_DYLIB: - libs.insert(find_dylib_name(obj_file, cmd)); - break; + struct mach_header_64 header; + fread(&header, sizeof(struct mach_header_64), 1, obj_file); + if(!(header.magic == MH_MAGIC_64 || header.magic == MH_CIGAM_64)) { + std::cerr << "Not a mach-o object file: " << filename << std::endl; + return std::set(); } - fseek(obj_file, cmd.cmdsize - (sizeof(uint32_t) * 2), SEEK_CUR); - } - fclose(obj_file); - libs.erase(filename); - return libs; + std::set libs; + for(uint32_t i = 0; i < header.ncmds; i++) { + struct load_command cmd; + fread(&cmd.cmd, sizeof(cmd.cmd), 1, obj_file); + fread(&cmd.cmdsize, sizeof(cmd.cmdsize), 1, obj_file); + switch(cmd.cmd) { + case LC_LOAD_DYLIB: + case LC_LOAD_UPWARD_DYLIB: + case LC_REEXPORT_DYLIB: + libs.insert(findDylibName(obj_file, cmd)); + break; + } + fseek(obj_file, cmd.cmdsize - (sizeof(uint32_t) * 2), SEEK_CUR); + } + fclose(obj_file); + libs.erase(filename); + return libs; } bool isSymlink(const Path & path) { - struct stat st; - if(lstat(path.c_str(), &st)) - throw SysError(format("getting attributes of path ‘%1%’") % path); + struct stat st; + if(lstat(path.c_str(), &st)) + throw SysError(format("getting attributes of path ‘%1%’") % path); - return S_ISLNK(st.st_mode); + return S_ISLNK(st.st_mode); } Path resolveSymlink(const Path & path) { char buf[PATH_MAX]; ssize_t len = readlink(path.c_str(), buf, sizeof(buf) - 1); if(len != -1) { - buf[len] = 0; - return Path(buf); + buf[len] = 0; + return Path(buf); } else { - throw SysError(format("readlink('%1%')") % path); + throw SysError(format("readlink('%1%')") % path); } } -std::set resolve_tree(const Path & path, PathSet & deps) { - std::set results; - if(deps.find(path) != deps.end()) { - return std::set(); - } - deps.insert(path); - for (auto & lib : runResolver(path)) { - results.insert(lib); - for (auto & p : resolve_tree(lib, deps)) { - results.insert(p); +std::set resolveTree(const Path & path, PathSet & deps) { + std::set results; + if(deps.find(path) != deps.end()) { + return std::set(); } - } - return results; + deps.insert(path); + for (auto & lib : runResolver(path)) { + results.insert(lib); + for (auto & p : resolveTree(lib, deps)) { + results.insert(p); + } + } + return results; } -std::set get_path(const Path & path) { - Path cacheFile = resolveCacheFile(path); - if(pathExists(cacheFile)) { - return readCacheFile(cacheFile); - } +std::set getPath(const Path & path) { + Path cacheFile = resolveCacheFile(path); + if(pathExists(cacheFile)) { + return readCacheFile(cacheFile); + } - std::set deps; - std::set paths; - paths.insert(path); + std::set deps; + std::set paths; + paths.insert(path); - Path next_path = Path(path); - while(isSymlink(next_path)) { - next_path = resolveSymlink(next_path); - paths.insert(next_path); - } + Path next_path = Path(path); + while(isSymlink(next_path)) { + next_path = resolveSymlink(next_path); + paths.insert(next_path); + } - for(auto & t : resolve_tree(next_path, deps)) { - paths.insert(t); - } + for(auto & t : resolveTree(next_path, deps)) { + paths.insert(t); + } - writeCacheFile(cacheFile, paths); + writeCacheFile(cacheFile, paths); - return paths; + return paths; } int main(int argc, char ** argv) { return handleExceptions(argv[0], [&]() { - initNix(); + initNix(); - struct utsname _uname; + struct utsname _uname; - uname(&_uname); + uname(&_uname); - cacheDir = (format("%1%/dependency-maps/%2%-%3%-%4%") - % settings.nixStateDir - % _uname.machine - % _uname.sysname - % _uname.release).str(); + cacheDir = (format("%1%/dependency-maps/%2%-%3%-%4%") + % settings.nixStateDir + % _uname.machine + % _uname.sysname + % _uname.release).str(); - auto store = openStore(); + auto store = openStore(); - auto drv = store->derivationFromPath(Path(argv[1])); - Strings impurePaths = tokenizeString(get(drv.env, "__impureHostDeps")); + auto drv = store->derivationFromPath(Path(argv[1])); + Strings impurePaths = tokenizeString(get(drv.env, "__impureHostDeps")); - std::set all_paths; + std::set all_paths; - for (auto & path : impurePaths) { - for(auto & p : get_path(path)) { - all_paths.insert(p); + for (auto & path : impurePaths) { + for(auto & p : getPath(path)) { + all_paths.insert(p); + } } - } - std::cout << "extra-chroot-dirs" << std::endl; - for(auto & path : all_paths) { - std::cout << path << std::endl; - } - std::cout << std::endl; + std::cout << "extra-chroot-dirs" << std::endl; + for(auto & path : all_paths) { + std::cout << path << std::endl; + } + std::cout << std::endl; }); }