C言語で自作したcpコマンドが上手く動作しません

質問

当方、プログラミングを勉強中の学生です。

先日、ファイル入出力関数を用いてcpコマンドを自作しました。
一応、コンパイルは通るのですが、コピーしたファイルを開くことができません。

そのファイルのパーミッションを確認してみたところ
「----------」となっており、読み書き実行すべて不可となっていました。

ソースは以下の通りなのですが、何が問題でしょうか。
回答よろしくお願い致します。

#include<stdio.h>
#include<fcntl.h>
#include <string.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/uio.h>
#include <unistd.h>

#define SIZE 8192

int main(int argc, char *argv[])
{
int fd1, fd2;
char buf[SIZE];

if ( argc != 3 ){
char err_message[] = "ファイル名を指定して下さい。\n";
write(2, err_message, strlen(err_message));
return 1;
}

argv[0] = "mycopy";
fd1 = open(argv[1], O_RDONLY);
fd2 = open(argv[2], O_WRONLY | O_CREAT);

if (fd1 < 0 || fd2 < 0) {
char err_message[] = "ファイルをオープンできません。";
write(2, err_message, strlen(err_message));
write(2, strerror(errno), strlen(strerror(errno)));
write(2, "\n", 1);
return 1;
}

while(1) {
if (read(fd1, buf, SIZE) == 0) {
break;
} else if (read(fd1, buf, SIZE) > 0) {
write(fd2, buf, SIZE);
} else {
char err_message[] = "エラーが発生しました。";
write(2, err_message, strlen(err_message));
write(2, strerror(errno), strlen(strerror(errno)));
write(2, "\n", 1);
return 1;
}

}
close(fd1);
close(fd2);
return 0;
}

回答No.4

entree · ベストアンサー

> そのファイルのパーミッションを確認してみたところ
> 「----------」となっており、読み書き実行すべて不可となっていました。

openは第三引数にパーミッションを指定できます。0666を設定すると期待した動作になるのではないでしょうか。（umaskが022なら、作成されるファイルのパーミッションは644になります）

その他の点について。

1.fd1, fd2をopenしてからエラーチェックしていますが、fd1のオープンに失敗してfd2のオープンが成功すると、O_CREATがあるために空ファイルができてしまう。
　==> 面倒でもエラーチェックは２つに分けるべきでしょう。
2.fd2のopen時にO_TRUNCがないと、ファイルがあった場合の動作がおかしいことになる。
　==> fd1のファイルサイズ < fd2のファイルサイズのファイルが存在する状況を作って、やってみるとわかると思います。read側の内容がabc、write側の内容がhijklmnなら、実行後のwrite側の内容はabcklmnになってしまいますよね？　期待される動作はabcのはずです。
3.read, writeのエラーチェックでEINTR（割り込み）に対する考慮がされていない。
　==> EINTRの場合はエラーとはみなさずリトライする処理が必要です。
4.read成功時の読み取りバイト数がSIZEであるという仮定が置かれている。
　==> 実際にはそれよりも少ないこともあり得ます。
5.nun00nunさんのコードでは、write成功時の書き込みバイト数がSIZEであるという仮定が置かれている。
　==> 実際にはそれよりも少ないこともあり得ます。その場合は残りの部分の書き出し処理が必要です。

以下、私の方で修正したコードです。

#include <stdio.h>
#include <fcntl.h>
#include <string.h>
#include <errno.h>
#include <unistd.h>

#define SIZE 8192

int
main (int argc, char *argv[])
{
　int fd1, fd2;
　char buf[SIZE], *bufp;
　int rsize, wsize;

　if (argc != 3)
　　{
　　　char err_message[] = "ファイル名を指定して下さい。\n";
　　　write (2, err_message, strlen (err_message));
　　　return 1;
　　}

　argv[0] = "mycopy";
　fd1 = open (argv[1], O_RDONLY);

　if (fd1 < 0)
　　{
　　　char err_message[] = "ファイルをオープンできません。";
　　　write (2, err_message, strlen (err_message));
　　　write (2, strerror (errno), strlen (strerror (errno)));
　　　write (2, "\n", 1);
　　　return 1;
　　}

　fd2 = open (argv[2], O_WRONLY | O_CREAT | O_TRUNC, 0666);

　if (fd2 < 0)
　　{
　　　char err_message[] = "ファイルをオープンできません。";
　　　write (2, err_message, strlen (err_message));
　　　write (2, strerror (errno), strlen (strerror (errno)));
　　　write (2, "\n", 1);
　　　return 1;
　　}

　while (1)
　　{
　　　bufp = buf;

　　　rsize = read (fd1, bufp, SIZE);

　　　if (rsize == -1)
　　　　{
　　　　　if (errno == EINTR)
　　　　　　{
　　　　　　　continue;
　　　　　　}

　　　　　char err_message[] = "読み込みエラーが発生しました。";
　　　　　write (2, err_message, strlen (err_message));
　　　　　write (2, strerror (errno), strlen (strerror (errno)));
　　　　　write (2, "\n", 1);
　　　　　return 1;
　　　　}

　　　if (rsize == 0)
　　　　{
　　　　　break;
　　　　}

　　　while (1)
　　　　{
　　　　　wsize = write (fd2, bufp, rsize);

　　　　　if (wsize == -1)
　　　　　　{
　　　　　　　if (errno == EINTR)
　　　　　　　　{
　　　　　　　　　continue;
　　　　　　　　}

　　　　　　　char err_message[] = "書き込みエラーが発生しました。";
　　　　　　　write (2, err_message, strlen (err_message));
　　　　　　　write (2, strerror (errno), strlen (strerror (errno)));
　　　　　　　write (2, "\n", 1);
　　　　　　　return 1;
　　　　　　}

　　　　　bufp += wsize;
　　　　　rsize -= wsize;

　　　　　if (rsize == 0)
　　　　　　{
　　　　　　　break;
　　　　　　}
　　　　}
　　}

　close (fd1);
　close (fd2);
　return 0;
}

お礼

お礼が遅くなってしまい失礼しました。
訂正したコードまで付けてくださるとは（泣）
勉強させていただきます。
ありがとうございました！

回答No.3

trapezium

細いことだけど。

> fd1 = open(argv[1], O_RDONLY);
> fd2 = open(argv[2], O_WRONLY | O_CREAT);
> if (fd1 < 0 || fd2 < 0) {

fd1 が失敗しても、fd2 が成功した段階で errno がクリアされるので、その下のエラーメッセージが正しくないときがある。

> write(2, strerror(errno), strlen(strerror(errno)));

その都度処理するか、errno を保存しておく。
まあ、err(1, なんたらでいいじゃないかと思わないこともないが、stdio 使わないことに意味があるんだろうと write(2, に関しては…

お礼

お礼が遅くなってしまい失礼しました。
回答ありがとうございます。勉強になりました！

回答No.2

Tacosan

ついでにいうと, read の返り値を捨てちゃうのもまずかったような.

お礼

おっしゃるとおりですね。プログラムを修正させていただきました。
回答ありがとうございました！

回答No.1

f272

fd2 = open(argv[2], O_WRONLY | O_CREAT);
の第3引数にパーミッションを指定してみたらどうかな？

それよりも
whileの中で
if (read(fd1, buf, SIZE) == 0) {
でfd1から読んで，また
} else if (read(fd1, buf, SIZE) > 0) {
でfd1から読むのは意図と違う気がする。

お礼

回答ありがとうございます。
ご指摘の通り、プログラム自体もよろしくありませんでした（＾＾：）
参考にさせていただきます！