有很多错误。
-
在main 中,filename 将指向所有文件的相同 地址。我们必须提供一个唯一地址/副本。一种方法是使用strdup。并且,我们应该在copyFile 的底部添加一个free 调用来释放strdup 分配的存储空间。
-
复制时我们必须跳过.和..。否则,代码段错误。
-
对输入文件和副本使用相同的目录可能会导致相当于无限循环。也就是说,给定一个复制到xyzcopy 的文件xyz,readdir 可能能够看到xyzcopy 作为输入并尝试创建xyzcopycopy。
最好使用单独的输出目录。那将需要进行重大改写。但是,更简单的解决方法是跳过任何包含 copy 的文件。
-
程序将[尝试]复制目录条目,即使它们不是普通文件(例如,它会尝试在目录上执行open)。我们应该检查文件类型并跳过非文件类型。
-
在copyFile 中,即使在main 中使用strdup,字符串filename 确实没有有足够的空间来容纳"copy" 的strcat。我们需要一个单独的缓冲区来存放输出文件名。
-
lseek/write 在输出文件的末尾添加了一个多余的二进制零。这是为了扩展输出文件。最好使用ftruncate。
-
这是因为将write 与mmap 混合可能会出现问题,并且通常不是最佳做法。在您的用例中,它可能没问题(因为write 是在mmap 之前完成),但如果我们麻烦使用mmap ,最好只使用mmap缓冲区指针和memcpy。
-
copyFile 缺少 return。而且,main 还缺少一个 return
-
copyFile 中的in 没有close。
-
open、mmap、opendir 调用没有错误检查。
-
在close 调用之前应该有munmap 调用。
错误未已修复:
-
程序限制目录可以有多少条目。
-
使用pthread_t threads[50];,我们可以仅在目录中有 50 个条目。 没有检查这个数组是否溢出。
-
并行处理所有个文件没有实际意义。这不缩放。
-
如果目录中有(例如)1,000,000 个条目,我们可能会耗尽资源。我们可能会用完未使用的进程槽,并且pthread_create 将在达到系统范围限制后开始失败。我们可能会用完文件描述符,并且在打开大约 1024 个文件后,open 调用将开始失败。
-
在创建了一定数量的线程(例如,实际上是 4-5 个)后,复制过程实际上会更慢,因为系统会花费大部分时间在线程之间切换而不是做有用的工作。
-
对于小文件,线程创建/拆除的开销相对于线程的运行时间变得很重要。
最好有一个有限的“工作”线程“池”,并将pthread_create 调用与pthread_join 调用穿插,在线程完成时回收/重用现在未使用/空闲的“槽”。
一种更有效的方法是创建线程池(通过pthread_create 在开始时调用一次。每个线程都有一个邮箱/队列要做。main 可以将条目排入线程队列,当它看到线程空闲时添加一个新条目。pthread_join 调用可以在最后完成一次
以下是更正后的代码。
我使用预处理器条件来表示旧代码与新代码(例如):
#if 0
// old code
#else
// new code
#endif
#if 1
// new code
#endif
无论如何,这是更正后的代码。我已经用 cmets 注释了错误/修复:
#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <dirent.h>
#include <pthread.h>
#include <fcntl.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/mman.h>
#include <string.h>
void *
copyFile(void *arg)
{
// NOTE/BUG: no cast is needed from a void *
#if 0
char *filename = ((char *) arg);
#else
char *filename = arg;
#endif
// NOTE/BUG: there is _not_ enough space in filename to add "copy" -- this is
// UB (undefined behavior)
#if 0
char *destname = strcat(filename, "copy");
#else
char destname[1024];
strcpy(destname,filename);
strcat(destname,"copy");
#endif
int in = 0;
in = open(filename, O_RDONLY);
// NOTE/FIX: check for error
#if 1
if (in < 0) {
perror(filename);
exit(1);
}
#endif
int out = 0;
out = open(destname, O_RDWR | O_CREAT | O_TRUNC,
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
// NOTE/FIX: check for error
#if 1
if (out < 0) {
perror(destname);
exit(1);
}
#endif
struct stat statbuf;
fstat(in, &statbuf);
off_t insize = statbuf.st_size;
// NOTE/BUG: this will add a binary zero at the end of the output file
#if 0
lseek(out, insize - 1, SEEK_SET);
ssize_t wrote = write(out, "", 1);
#else
ftruncate(out,insize);
#endif
char *inmap = mmap(0, insize, PROT_READ, MAP_FILE | MAP_PRIVATE, in, 0);
// NOTE/FIX: check for error
#if 1
if (inmap == MAP_FAILED) {
perror("mmap/in");
exit(1);
}
#endif
char *outmap = mmap(0, insize, PROT_READ | PROT_WRITE,
MAP_FILE | MAP_SHARED, out, 0);
// NOTE/FIX: check for error
#if 1
if (inmap == MAP_FAILED) {
perror("mmap/in");
exit(1);
}
#endif
memcpy(outmap, inmap, insize);
// NOTE/FIX: we should unamp the output area
#if 1
munmap(outmap,insize);
#endif
close(out);
// NOTE/FIX: we should unmap and close the input descriptor
#if 1
munmap(inmap,insize);
close(in);
#endif
// NOTE/FIX: free the string allocated by strdup in main
#if 1
free(filename);
#endif
// NOTE/FIX: needs return value -- would be flagged by compiler if compiled
// with -Wall
#if 1
return (void *) 0;
#endif
}
int
main(int argc, char *argv[])
{
DIR *d;
struct dirent *e;
// NOTE/BUG: no checks for missing argument or bad directory
#if 0
d = opendir(argv[1]);
#else
if (argv[1] == NULL) {
fprintf(stderr,"missing argument\n");
exit(1);
}
d = opendir(argv[1]);
if (d == NULL) {
perror(argv[1]);
exit(1);
}
#endif
// NOTE/BUG: unused variable
#if 0
int num_entries = 0;
#endif
int i = 0;
pthread_t threads[50];
while ((e = readdir(d)) != NULL) {
// NOTE/FIX: must skip "." and ".."
#if 1
if (strcmp(e->d_name,".") == 0)
continue;
if (strcmp(e->d_name,"..") == 0)
continue;
#endif
// NOTE/FIX: only copy simple files (i.e. do _not_ copy subdirectories, etc.)
#if 1
if (e->d_type != DT_REG)
continue;
#endif
// NOTE/FIX: skip "copy" files
#if 1
if (strstr(e->d_name,"copy") != NULL)
continue;
#endif
// NOTE/BUG: this will present the _same_ memory address to all threads so there
// is a "race condition"
#if 0
char *filename = e->d_name;
#else
char *filename = strdup(e->d_name);
#endif
printf("%s\n", filename);
pthread_create(&threads[i], NULL, copyFile, filename);
i++;
}
for (int j = 0; j < i; j++) {
pthread_join(threads[j], NULL);
}
// NOTE/FIX: needs return
#if 1
return 0;
#endif
}