|
>>4380 DNS勉強中 > length の件ですが、もうしわけありませんが一次資料は存じておりません。 了解しました。一次資料を発見されたら、ご教示いただけると幸いです。 > そのため、このサイトで有効利用していただけるのでしたらありがたいです。 ありがとうございます。C でのサンプルソースとしてぜひ使わせていただければ と思います。ただ、今は時間がとれないので、半年後とか 1年後、もしかしたら 数年後となるかもしれません。その点はご了承ください。 > resolver-1.pl を C に移植しようとしたものがこのソースです。 そうなんですか。わたしはこんなわかりやすいソースを書いた覚えはないんですが、 とても可読性が高いように感じます。 なぜでしょうね? コメントが充実しているから? わたしは「適切な変数名・関数名を 使用していれば、あまりコメントを書かなくても問題ない」という考え方だった のですが、ちょっと考えが変わりそうです。 |
|
人材募集です。ご興味を持たれた方はお気軽にどうぞ。 http://X68000.q-e-d.net/~68user/tmp/job.html |
|
>>4381 68userさん 今回作成したソースは resolver-1.pl を参考に書かせていただきましたが、 どちらかというと書かれているソースではなく、 書かれているコメントを参考に作成しました。 理由は単に perl が分からないからです(^^;。 ただ、コメントから行っている動作が分かれば、 それを C で実現するにはどうすればよいかを考えれば良いのかなと。 以上のことより、 resolver-1.pl と同じような事を実現していても ソース自体は似つかないものになっているかもしれません。 と言いますか perl が読めないので「似てるか否かわからない」 というのが実際のところです(^^;。 後、ソースを利用する／しない、時期などはすべてお任せします。 それとごみデータの件ですが、下記の記述でうまくいきました。 sizeof(DNS_Send) - sizeof(DNS_Send.QUERIES) + count + 4 アドバイスを頂き、どうもありがとうございました。 |
|
投稿した DNS のソースコードの問題点 その1 ( Linux版のみ )照会タイプと照会クラスを共に指定しないと、 照会クラスに PWD=/root/src が挿入され (null にならず)、エラーになってしまう。 その部分にだけ絞ったサンプルソース( Windows/Linux 兼用 ) http://kansai2channeler.hp.infoseek.co.jp/cgi-bin/joyful/img/1057.c windowでの実行結果 argv[0] = C:\src\argv.exe(←これは私の実行ファイル名です) argv[1] = (null) argv[2] = (null) argv[3] = (null) argv[4] = (null) linuxでの実行結果 argv[0] = ./test24(←これは私の実行ファイル名です) argv[1] = (null) argv[2] = PWD=/root/src argv[3] = HOSTNAME=gifu-vm-35 argv[4] = LESSOPEN=|/usr/bin/lesspipe.sh %s 思いつく回避方法は、 1,照会タイプを入力必須にする。 2,照会クラスのみ引数ではなく、別途入力するようにする。 3,照会クラスの先頭3文字が PWD なら INTERNET に置き換えてしまう。 と言った所ですが、1,2は resolver-1.pl にくらべ多く入力する必要があり、 3は環境依存度が高いかも知れません。 このサイトで公開される際は、この部分は何かしか改善を加えた方が良いかもしれません。 |
|
>>4384 DNS勉強中 > 後、ソースを利用する／しない、時期などはすべてお任せします。 ありがとうございます。有効に活用させていただきたいと思います。再配布 ライセンスやクレジット表記について要望があれば、今のうちに承っておきます。 > printf("argv[0] = %s\n", argv[0]); > printf("argv[1] = %s\n", argv[1]); C の話になりますが、argv の要素数は argc を参照しなければいけません。領域を 超えた部分に何が入っているかは環境依存です。Windows で NULL が入っているのは たまたまです。OS のバージョンやコンパイラを変えたらどうなるかわかりません。 Linux の場合は、環境変数を保持している environ の環境にアクセスしていますが、 これもたまたまです。 # この領域は、普通は int main(int argc, char *argv[], char *envp[]) # などとアクセスします。 よって、 char query_type[256]; char query_class[256]; if ( argc >= 4 ){ strcpy(query_type, argv[3]); } else { strcpy(query_type, "A"); } if ( argc >= 5 ){ strcpy(query_class, argv[4]); } else { strcpy(query_class, "INTERNET"); } や char *query_type = "A"; char *query_class = "INTERNET"; if ( argc >= 4 ) query_type = argv[3]; if ( argc >= 5 ) query_class = argv[4]; などとするのがよいでしょう。 |
|
>>4385 68userさん ライセンスなどは不要です。 それと引数の御指導どうもありがとうございます。 よく理解でき、Linuxでの動作も確認いたしました。 おかげで問題点が1つ解消されました。 後、パディングが心配との事で、構造体を書いていただきましたので、 それを参考にソースを書き直してみました。 Windows版 http://kansai2channeler.hp.infoseek.co.jp/cgi-bin/joyful/img/1058.txt ただ struct DNS_Query { .... }; の .... の意味するところが分からなかったので ここは私なりに解釈して記述しています。 またstruct DNS_Query *q;の必要性が分からなかったので、これは記述していません。 しかし、 > 問い合わせ内容を DNS_Question に構築し、 > 送信時にDNS_Packet にコピーして send すると思います。 の部分は実現したつもりです。 もっともパディングが心配という事が良く理解できていないので、 果たして組んだソースが意図を汲んでいるかどうかは自信がありません。 後このソースの問題点ですが、いま自分が気づいているところでは2点あります。 変数 search と 変数 DNS_question.flag の保守性です。 動作自体には問題はないと思います。 ただこれは自分で直せる気がするので週末にでも直したいと思います。 |
|
>>4379 68user ありがとうございます。 おかげさまでつい先ほど本番サーバへ移行が完了しました。 本番サーバではリモートコマンドはできないポリシーになっていたようなのでftp でtarファイルを転送し、rootで展開しました。 |
|
>>4386 DNS勉強中 > ここは私なりに解釈して記述しています。 わたしの意図どおりの実装になっています。 > またstruct DNS_Query *q;の必要性が分からなかったので、これは記述していません。 struct DNS_Question { short id short flag; short QDCOUNT; short ANCOUNT; short NSCOUNT; short ARCOUNT; struct DNS_Query *query; }; と書くと、DNS_Question というものは 「id・flag・{QD,AN,NS,AR}COUNT・DNS_Query から成り立つ」 という関係がソース上に明示されます。プログラムは「データ構造＋アルゴリズム」 ですので、正確なデータ構造を最初に明示しておけば、ソースを理解しやすくなる ということです。 また、こうしておけば、 main(){ make_question(&DNS_question); } make_question(DNS_Question *question){ question->id = ...; question->flag = ...; make_query(question->query); } make_query(DNS_Query *query){ query を作成 } というふうに、変数の受け渡しがすっきりします。 ただ、フラットな構成の方がわかりやすい場合もありますので、ケースバイケースかと 思います。 > もっともパディングが心配という事が良く理解できていないので、 わたしの環境では int main (){ struct { char a[1]; int b; } hoge; printf("%d\n", sizeof(hoge)); } の結果は 8 になります。構造体のメンバ間にどれだけのパディングが入るかは 環境依存なので、パディングが入らないことを前提としたコーディングをするのは 怖いなぁ、ということです。 > 後このソースの問題点ですが、いま自分が気づいているところでは2点あります。 > 変数 search と 変数 DNS_question.flag の保守性です。 なるほど、学校の課題にも関わらず細かな部分の完成度を高めたいというのは 素晴らしいですね。余計なお世話かもしれませんが細かいところまで見てみました。 > #include <netdb.h> 必要なヘッダファイルを include していません。その結果、引数チェックが 行われず、戻り値は int として扱われているはず。 このことに気づかないということは、警告オプションを有効にしていないと推察 されます。gcc であれば -Wall オプションを付けると幸せな人生を送れます。 > char *make_domain(char *domain_name); プロトタイプ宣言で変数名は不要で、 char *make_domain(char *); でよいです。別に引数名を記述しても動作には影響ないですが、引数名を変更 したときに直し忘れてズレが出るので、わたしは書きません。/usr/include/*.h にも書いてないです。 > fprintf(stderr, "Error: Socket作成失敗\n"); なぜ失敗したのかがわからないため、システムコールのエラー時には errno を 出力しましょう。わたしは fprintf(stderr, "Error: Socket作成失敗。errno[%s]\n", strerror(errno)); とするのが好みですが、perror(3) などを使う人も多いです。 bind 失敗時も同様。エラーメッセージが Bind error であるのと Bind error [Address already in use] と表示されるのでは、対処時間に大きな差が出ると思います。 ちなみに gethostbyname(3) のエラー処理が抜けています。gethostbyname は ソケットまわりのシステムコール呼び出し時のエラー以外にも、データ内容の エラーが発生する可能性があるので herror(3) や hstrerror(3) を使います。 > return -1; main で戻せる値は 0〜255 です。あと、exit と return が混在しているのが 気になります。 > questions = two_bytes(int_buf[search], int_buf[search+1]); > search += 2; two_bytes・four_bytes のインタフェースがいまいちです。データ取得と インクリメントを別々に行うと、インクリメント忘れ・インクリメントミスが 発生する危険性があります。 たとえば questions = two_bytes(&int_buf, &search); とインタフェースを変え、 int two_bytes(int *int_buf, int *search){ int sum; sum = int_buf[*search] * 16 * 16 + int_buf[*(search+1)]; *search += 2; return sum; } とすればデータ解析とインクリメントを同時に行えます。ここまでくれば、 struct DNS_Response { char data[10000]; int int_data[10000]; int pos; }; という構造体を作った方がよいかもしれません。 上記と関連しますが、 additional = two_bytes(&int_buf[search], &search); search += 1; これは一見すると謎なコードです。2バイト取得しているのに 1バイトしか進めて いないので、わたしはしばらく悩みました。結局は get_domain() の先頭で 1バイト 進めているから問題ないのですが、直感に反するコーディングだと思います。 > int conv_class(char *class){ > switch (class) { > case 1: printf("1 (INTERNET)\n"); break; > case 2: printf("2 (CSNET)\n"); break; > case 3: printf("3 (CNAME)\n"); break; > case 4: printf("4 (HESIOD)\n"); break; > case 5: printf("5 (ANY)\n"); break; > } わたしなら、 typedef struct { char name[100]; int code; } codelist_t; としておいて、 int conv_class(char *class){ codelist_t class_list[] = { {"INTERNET", 1}, {"CSNET", 2}, {"CHAOS", 3}, {"HESIOD", 4}, {"ANY", 5}, }; for ( i=0 ; i<sizeof(class_list)/sizeof(class_list[0]) ; i++ ){ if ( strcmp(class, class_list[i].str) == 0 ){ return class_list[i].code; } } と直します。class_list は void class_print(int class) で共用してもよいでしょう (resolver-1.pl と同じやり方)。 > memset(&client, '\0', sizeof(client)); > host[j] = 0; /* 最後に0を代入する */ memset は 0 で埋めて、host[j] は '\0' で終端すべきです。文字終端の '\0' と 値としての 0 を区別しておくと、後から読んだ人に意図が伝わりますので。 あとは命名について。関数名は動詞、変数名は名詞という基本ルールがあるよう ですが、徹底されていないように思います。 いまいちだと思う関数名: int domain_count(char *); → get_domain_length などが適切では。 int end_domain(int, int *); → is_XXX とか check_XXX がよいのでは。 int resource_data(int *, int, int, char *); → 名詞になっている。しかし他の hogehoge_print より一段レベルが高いので、 混同しないような命名をしたいところ。 void conv_time(int); → 「変換」といいつつ実際に行っているのは「出力」。 いまいちだと思う変数名: int search; → 動詞に見える。pos あたりでいいのでは。 int questions; int answer; int authority; int additional; → s が付いていたり付いていなかったり。あと、この変数の本質は「個数」で あるはず。であれば question_count とか question_num などにしたい。 int flag = 0; → flag や tmp という変数名が許されるのは変数スコープが 5行以内の場合 (わたしの中では)。それ以上ならもう少し長い名前を。 > int resource_data(int *int_buf, int search, int max, char *print); int max → record_num あたりで。 char *print → record_name あたりで。 > int domain_count(char *argv) argv は渡す側の変数名であって、この引数の本質は「ホスト名」であるため、 int domain_count(char *hostname) int domain_count(char *host) などがよいと思います。 と、わたしならこう書くという部分はあるものの、それでも可読性が高い と感じました。たとえば codelist_t は保守性は高いのですが、ぱっと見 読みやすいのは case 文の羅列です。勉強用のソースであれば、下手に 構造化しすぎることなく、あえてベタ書きしたほうがいいのではないかと 考え始めています。 あと、hoge_print 関数も、わたしはこういう書き方をせずに char *a_record_print(int_buf, search); と文字列を返すほうですが、メモリ確保を行うのは呼び出し側か、関数の 中で static に持つか、など悩ましいです。すっぱり割り切って関数内で printf するのも、保守性はともかく可読性は悪くない、と思いました。 あと、 - two_bytes・four_bytes という関数は便利で読みやすい。 - int_buf にコピーするのは、型変換のバグを避ける手法としては 優れているのかも (ちょっと逃げっぽいけど)。 - perl 版の可読性を落としているのはやはり pack。num2bin などの 関数を作り、直接 pack を呼ぶのは避けた方がよいのかも。 - やはり C は構造体がすっきり書けるのがよいところ。perl の場合、 ハッシュだとタイプミス耐性がないし、Class::Struct だとインクリメント が面倒だし、package を作るのは大げさ。 などといろいろと思うところはありました。自分が書いたものを他人が リライトすると、いろいろと気づくことがあっておもしろいですね。 いろいろ偉そうに書きましたが、指摘したことは当ページでは全然実践できて いません (エラーチェックは甘いし、herror(3) は使っていないし、resolver-1.pl ではデータ取得とインクリメントを別々に行っているし)。その辺はご容赦ください。 |
|
>>4388 68userさん いろいろと細かい点までアドバイスをしていただきましてありがとうございます。 大変勉強になります。 全てに指摘点に対しご回答差し上げたいのですが、 まだ指摘された部分の修正作業を行っておりませんので、 一通り読ませていただき、気づいた点について記述させていただきます。 まず return と exit の混在ですが、 これは conv_type 関数や conv_class 関数の部分だけを 指した指摘でしょうか。 それとも exit を一つも使わない方が良いという事でしょうか。 return と exit の混在のデメリットが分からなかったので。 後、変数 search ですが、仰るとおりインターフェースがいまいちです。 何故なら現在位置を返す関数と返さない関数が混在しているからです。 ここの処理がうまく思いつかなかったので search += 2; で逃げていました(^^;。 そして変数 search の大きな問題が仰るとおり > additional = two_bytes(&int_buf[search], &search); > search += 1; です。 ここは作った当人で無いと分からないですよね(^^;。 その理由は、開発途中で方針を変更してしまったためです。 そのため現在位置は ・解析し終わった部分の、次の部分の先頭に現在位置を合わせている所 ・解析し終わった部分の末尾に現在位置を合わせている所 の2パターンが発生してしまいました。 その代償が search += 2; と search += 1; の混在です。 ここは人に解析してもらう際にはまずいので修正します。 あと host[j] = 0; ですが、ここでは www.livedoor.com というドメイン名を 3www8livedoor3com0(数字の部分は文字ではなく、文字コード)に変換しているのですが、host[j] = 0; は ドメインの最後に文字コードの 0 を連結する と言う意味で記述しました。 何となく思ったのですが、 3www8livedoor3com0 の最後の 0 は終端という意味かな。 そう考えると仰られている意味が分かりました。 その部分に関しては、記述ミスと言うより、私の理解が間違っていたのかなと思います。 それと、int_buf は完全に逃げです(^^;。 ここの部分のうまい処理が思いつきませんでした。 他の指摘された部分につきましても、順次修正して良いものを作りたいと思います。 お忙しい中、いろいろご指摘いただきありがとうございます。 |