(コードレビュー)(url解析機)[雑談]

フォーラム(掲示板)ルール
フォーラム(掲示板)ルールはこちら  ※コードを貼り付ける場合は [code][/code] で囲って下さい。詳しくはこちら
ぷぷぷジューシー
記事: 28
登録日時: 3年前
住所: アンドロメダ銀河系のどこかに住んでいる

(コードレビュー)(url解析機)[雑談]

#1

投稿記事 by ぷぷぷジューシー » 3年前

ホスト名とパス名をただ返すだけのプログラム
手抜きです
バグなど少しでもパフォーマンス改善できる所を教えてくれると嬉しいです
出来ればurl解析機バージョン0.1作りたいと思う
設計
1. 日本語文字列を配列にしまう
2. 配列にした文字列をfor分でワイド文字に変換する
3:1. "/'をカウントする
2. '/"二個になったらrhostを+=で増えるようにする
2:0. '/"が三個になったらcontinueで処理をスキップする
3. '/"が三個以上になったらrpathを+=で増えるようにする
4. 最後にリフレッシュする

参考にしたサイト
viewtopic.php?f=3&t=21106&sid=c2e081fe9 ... aaaeed9a47
https://programming-place.net/ppp/conte ... toc32.html
https://www.ibm.com/support/knowledgece ... m#mbrtoc32

環境 {
cコンパイラ: MinGW-W64 8.1.0,
os: windows 10 20hb,
ライブラリ{
stdio.h,
string.h,
uchar.h,
limits.h,
stdlib.h,
}
}

コード:

#include <stdio.h>
#include <string.h>
#include <uchar.h>
#include <limits.h>
#include <stdlib.h>


int geturlparserfunc(const char*,   char*, char*);

int main(void)
{
  char hostname, pathname;
  geturlparserfunc("", &hostname, &pathname);

  printf("ホスト名; %s\n", hostname);
  printf("パッチ名: %s\n", pathname);

}

int geturlparserfunc(const char* name,   char* rhost, char* rpath)
{
  size_t bmfunc_rlen;

  char32_t inp32b[MB_LEN_MAX * 5];
  int  inputlen = strlen(name) + 5;
  char input[(int)inputlen];
  int arraycount1 = 0, arraycount2 = 0;

  int slashconunt = 0, colon = 0;
  char temppath[MB_LEN_MAX * 5];
  char temphost[MB_LEN_MAX * 5];


  strcpy_s(input, inputlen+3, (char*)(name));
  input[inputlen] = '\0';

  for (int n = 0; inputlen > n ; ++n){
    bmfunc_rlen = mbrtoc32(&inp32b[arraycount1], &input[arraycount2],MB_LEN_MAX * 5, NULL);
    if (( (size_t)(0) == bmfunc_rlen ) ){
      break;
    } else if ( (size_t)(0) < bmfunc_rlen ){
    	arraycount1++;
      arraycount2 += (int)(bmfunc_rlen);
    } else if ( (size_t)(-3) == bmfunc_rlen ){
      arraycount1++;
      continue;
    } else if ( (size_t)(-1) == bmfunc_rlen ){
      printf("エラーが発生しました\n");
      printf("エンコードエラー又はバイト数が足りませんの可能性があります。");
      printf("エラーコード: %s\n", bmfunc_rlen);
      break;
    } else if ( (size_t)(-2) == bmfunc_rlen ){
      printf("エラーが発生しました\n");
      printf("マルチバイト文字の可能性があります。");
      printf("エラーコード: %s\n", bmfunc_rlen);
      break;
    }
  }

  for ( int n = 0; n < arraycount1; n++ ){
    if (inp32b[n] == ':' || inp32b[n] == '/'){
      if (inp32b[n] == ':'){
        colon = 1;
      }
      if (inp32b[n] == '/'){
        slashconunt += 1;
      }
    }

    if (colon == 1 && slashconunt == 2){
      // rhost 処理
      bmfunc_rlen = c32rtomb(temphost ,inp32b[n], NULL);
      if (bmfunc_rlen == -1){
        printf("エラーが発生しました\n");
        printf("有効なワイド文字ではありません\n");
        printf("エラーコード: %s\n", bmfunc_rlen);
        break;
      }

      *rhost += *(char*)(temphost);

      if (slashconunt == 3) {
        continue;
      }
    } else if (colon == 1 && slashconunt > 3) {
      bmfunc_rlen = c32rtomb(temppath ,inp32b[n], NULL);
      if (bmfunc_rlen == -1){
        printf("エラーが発生しました\n");
        printf("有効なワイド文字ではありません\n");
        printf("エラーコード: %s\n", bmfunc_rlen);
        break;
      }
      *rpath += *(char*)(temppath);
    }
  }

  arraycount1 = 0, arraycount2 = 0, slashconunt = 0, colon = 0 , bmfunc_rlen = 0;

  memset(inp32b, '\0', sizeof(inp32b)), memset(input, '\0', sizeof(input));

  return 0;
}
面倒なことはCGo使おう!

アバター
みけCAT
記事: 6734
登録日時: 13年前
住所: 千葉県
連絡を取る:

Re: (コードレビュー)(url解析機)[雑談]

#2

投稿記事 by みけCAT » 3年前

ぷぷぷジューシー さんが書きました:
3年前

コード:

int main(void)
{
  char hostname, pathname;
  geturlparserfunc("", &hostname, &pathname);

  printf("ホスト名; %s\n", hostname);
  printf("パッチ名: %s\n", pathname);

}
これはひどいですね。
printfの%sは文字列(終端にNUL文字がある文字の配列)を指すchar*型のデータを要求するのに対し、
hostnameとpathnameはchar型です。
これは未定義動作になり、一般的な環境ではアクセス違反になる可能性が高いでしょう。
さらに、「ホスト名とパス名を返す」はずなのに、
1文字(終端のNUL文字で埋まってしまい、文字列としては0文字)分の領域へのポインタを渡す、
というのは明らかにおかしいでしょう。
ポインタを渡して結果を格納させる場合、
・結果の文字列を格納するのに十分な領域(配列)へのポインタを渡す
・結果の文字列の位置を格納するポインタへのポインタを渡す
のどちらかにするべきでしょう。
複雑な問題?マシンの性能を上げてOpenMPで殴ればいい!(死亡フラグ)

アバター
みけCAT
記事: 6734
登録日時: 13年前
住所: 千葉県
連絡を取る:

Re: (コードレビュー)(url解析機)[雑談]

#3

投稿記事 by みけCAT » 3年前

ぷぷぷジューシー さんが書きました:
3年前
ホスト名とパス名をただ返すだけのプログラム
返しませんね。
まずは空文字列ではなくちゃんとURLを入力として与え、
求める結果が得られるプログラムにすることをおすすめします。
求める結果が得られずに困っているのであれば、そのような質問をすることをおすすめします。
ぷぷぷジューシー さんが書きました:
3年前
2. 配列にした文字列をfor分でワイド文字に変換する
for分ではなく、for文ですね。
ぷぷぷジューシー さんが書きました:
3年前
2. '/"二個になったらrhostを+=で増えるようにする
3. '/"が三個以上になったらrpathを+=で増えるようにする
C言語において、+=は文字列の結合ではなく数値(もしくはポインタ)の加算を行います。
文字列に文字列を結合(追加)するには、strcat関数を用いるのがいいでしょう。
(strcat関数の引数は両方「文字列」へのポインタでなければならないため、
文字だけを変換した後「文字列」にするためにNUL文字を追加するのを忘れないようにしましょう)
ぷぷぷジューシー さんが書きました:
3年前
4. 最後にリフレッシュする
memsetをしてその後その値を使わない場合、最適化により除去される可能性があります。
どうせ参照されなくなる領域へのmemsetは不要なので削除するか、
セキュリティのためゼロクリアしたいのであれば、
対応環境ならmemset_sやSecureZeroMemory関数を使うべきです。
パスワードとmemset関数 - yohhoyの日記
ぷぷぷジューシー さんが書きました:
3年前

コード:

  strcpy_s(input, inputlen+3, (char*)(name));
実際のバッファはinputlen要素しか無いのに、要素数としてinputlen+3を渡しており、危険です。
バッファサイズは正直に申告しましょう。
ぷぷぷジューシー さんが書きました:
3年前

コード:

  input[inputlen] = '\0';
範囲外への書き込みであり、未定義動作を起こし、周辺のデータを破壊する可能性があり、危険です。
削除しましょう。
ぷぷぷジューシー さんが書きました:
3年前

コード:

    if (( (size_t)(0) == bmfunc_rlen ) ){
      break;
    } else if ( (size_t)(0) < bmfunc_rlen ){
bmfunc_rlenは符号なしであるsize_t型なので、0でなければ必ず0より大きいです。
したがって、この2個の条件式のどっちかが必ず真になり、これより後のif文は実行されません。
2個目の条件式が不適切でしょう。
他の特殊な値のチェックを先に持ってくるのがいいでしょう。
ぷぷぷジューシー さんが書きました:
3年前

コード:

      printf("エラーコード: %s\n", bmfunc_rlen);
printfの%sは文字列を指すchar*型のデータを要求するのに、size_t型のbmfunc_rlenが渡されており、
未定義動作になります。
size_t型のデータを出力するには、%zuを用います。
ぷぷぷジューシー さんが書きました:
3年前

コード:

      *rhost += *(char*)(temphost);
      *rpath += *(char*)(temppath);
temphostおよびtemppathはchar型の要素の配列なので、
式中ではsizeof演算子を使う場合などの例外を除いてその先頭要素へのポインタに変換されます。
これはchar*型なので、char*型へのキャストは無害ですが冗長です。
ぷぷぷジューシー さんが書きました:
3年前

コード:

      if (slashconunt == 3) {
        continue;
      }
ここではこの前のif文の条件によりslashconunt == 2なので、この条件式が真になることは無いはずです。
そもそも、変数名をslashcountではなくslashconuntとしているのは個性的ですね。
オフトピック
そもそも、URLにはマルチバイト文字は使えないはずなので、
わざわざマルチバイト文字の変換をかけるのは余計という考え方も…
URIで使用できる文字 - CyberLibrarian
複雑な問題?マシンの性能を上げてOpenMPで殴ればいい!(死亡フラグ)

ぷぷぷジューシー
記事: 28
登録日時: 3年前
住所: アンドロメダ銀河系のどこかに住んでいる

Re: (コードレビュー)(url解析機)[雑談]

#4

投稿記事 by ぷぷぷジューシー » 3年前

みけCAT さんが書きました:
3年前
いくつか改善したけど何故か出力しませんでした何が原因でしょうか?

コード:

#include <stdio.h>
#include <string.h>
#include <uchar.h>
#include <limits.h>
#include <stdlib.h>


int geturlparserfunc(const char*,   char*, char*);

int main(void)
{
  char hostname, pathname;
  geturlparserfunc("https://golang.org/", &hostname, &pathname);

  printf("ホスト名; %s\n", hostname);
  printf("パッチ名: %s\n", pathname);

}

int geturlparserfunc(const char* name,   char* rhost, char* rpath)
{
  size_t bmfunc_rlen;
  int  inputlen = strlen(name) + 5;
  char input[(int)inputlen];
  char32_t inp32b[(int)inputlen];
  int arraycount1 = 0, arraycount2 = 0;

  int slashconunt = 0, colon = 0;
  char temppath;
  char temphost;


  strcpy_s(input, inputlen, (char*)(name));
  input[inputlen] = '\0';

  while (1){
    bmfunc_rlen = mbrtoc32(&inp32b[arraycount1], &input[arraycount2],MB_LEN_MAX * 5, NULL);
    if ( (size_t)(-1) == bmfunc_rlen ){
      printf("エラーが発生しました\n");
      printf("エンコードエラー又はバイト数が足りませんの可能性があります。");
      printf("エラーコード: %zu\n", bmfunc_rlen);
      break;
    } else if ( (size_t)(-2) == bmfunc_rlen ){
      printf("エラーが発生しました\n");
      printf("マルチバイト文字の可能性があります。");
      printf("エラーコード: %zu\n", bmfunc_rlen);
      break;
    } else if ( (size_t)(-3) == bmfunc_rlen ){
      arraycount1++;
      continue;
    }  else if (( (size_t)(0) == bmfunc_rlen ) ){
      break;
    } else if ( (size_t)(0) < bmfunc_rlen ){
    	arraycount1++;
      arraycount2 += (int)(bmfunc_rlen);
    } 
  }

  int n = 0;
  while ( arraycount1 > n ){
    ++n;
    if (inp32b[n] == ':' || inp32b[n] == '/'){
      if (inp32b[n] == ':'){
        colon = 1;
      }
      if (inp32b[n] == '/'){
        slashconunt += 1;
      }
    }

    if (slashconunt == 3) {
      continue;
    }

    if (colon == 1 && slashconunt == 2){
      // rhost 処理
      bmfunc_rlen = c32rtomb(&temphost ,inp32b[n], NULL);
      if ((size_t)(-1) == bmfunc_rlen){
        printf("エラーが発生しました\n");
        printf("有効なワイド文字ではありません\n");
        printf("エラーコード: %zu\n", bmfunc_rlen);
        break;
      }

      *rhost += temphost;
    } else if (colon == 1 && slashconunt > 3) {
      bmfunc_rlen = c32rtomb(&temppath ,inp32b[n], NULL);
      if ((size_t)(-1) == bmfunc_rlen){
        printf("エラーが発生しました\n");
        printf("有効なワイド文字ではありません\n");
        printf("エラーコード: %zu\n", bmfunc_rlen);
        break;
      }
      printf("%s\n", temppath);
      *rpath += temppath;
    }
  }

  arraycount1 = 0, arraycount2 = 0, slashconunt = 0, colon = 0 , bmfunc_rlen = 0;

  memset(inp32b, '\0', sizeof(inp32b)), memset(input, '\0', sizeof(input));

  return 0;
}

面倒なことはCGo使おう!

アバター
みけCAT
記事: 6734
登録日時: 13年前
住所: 千葉県
連絡を取る:

Re: (コードレビュー)(url解析機)[雑談]

#5

投稿記事 by みけCAT » 3年前

まず、マルチバイト文字の変換エラーを起こすような入力ではないので、
geturlparserfunc関数内のエラーの出力は実行されません。
さらに、#2の指摘内容が修正されていないので、
printfの処理内容がバッファを抜けて出力に反映されるより前に落ちてしまい、出力がされないのでしょう。
char型は可変長引数として渡す時はintに変換されるので、printfで出力するには%d(など)を用います。

また、範囲外への書き込みを行う
ぷぷぷジューシー さんが書きました:
3年前

コード:

  input[inputlen] = '\0';
も残っていますね。未定義動作です。

さらに、足し算を行っている*rhostや*rpathは初期化されていないstaticでないローカル変数を
指しているポインタをデリファレンスしているものなので、不定の値への足し算であり、未定義動作になります。

また、この入力では実行されないようですが、
ぷぷぷジューシー さんが書きました:
3年前

コード:

      printf("%s\n", temppath);
も文字列を指すchar*型のデータを要求する%sに対し整数を渡しており、未定義動作になります。

さらに、これもこの入力では実害が出る可能性は低そうですが、
ぷぷぷジューシー さんが書きました:
3年前

コード:

      bmfunc_rlen = c32rtomb(&temphost ,inp32b[n], NULL);
      bmfunc_rlen = c32rtomb(&temppath ,inp32b[n], NULL);
では、MB_CUR_MAX以上の要素数が必要とされるc32rtomb関数の第1引数に
1要素しかない領域へのポインタを渡しており、危険である可能性があります。
複雑な問題?マシンの性能を上げてOpenMPで殴ればいい!(死亡フラグ)

ぷぷぷジューシー
記事: 28
登録日時: 3年前
住所: アンドロメダ銀河系のどこかに住んでいる

Re: (コードレビュー)(url解析機)[雑談]

#6

投稿記事 by ぷぷぷジューシー » 3年前

#2とはどこの所ですか
面倒なことはCGo使おう!

アバター
みけCAT
記事: 6734
登録日時: 13年前
住所: 千葉県
連絡を取る:

Re: (コードレビュー)(url解析機)[雑談]

#7

投稿記事 by みけCAT » 3年前

#2とはこれです。
みけCAT さんが書きました:
3年前
ぷぷぷジューシー さんが書きました:
3年前

コード:

int main(void)
{
  char hostname, pathname;
  geturlparserfunc("", &hostname, &pathname);

  printf("ホスト名; %s\n", hostname);
  printf("パッチ名: %s\n", pathname);

}
これはひどいですね。
printfの%sは文字列(終端にNUL文字がある文字の配列)を指すchar*型のデータを要求するのに対し、
hostnameとpathnameはchar型です。
これは未定義動作になり、一般的な環境ではアクセス違反になる可能性が高いでしょう。
さらに、「ホスト名とパス名を返す」はずなのに、
1文字(終端のNUL文字で埋まってしまい、文字列としては0文字)分の領域へのポインタを渡す、
というのは明らかにおかしいでしょう。
ポインタを渡して結果を格納させる場合、
・結果の文字列を格納するのに十分な領域(配列)へのポインタを渡す
・結果の文字列の位置を格納するポインタへのポインタを渡す
のどちらかにするべきでしょう。
複雑な問題?マシンの性能を上げてOpenMPで殴ればいい!(死亡フラグ)

ぷぷぷジューシー
記事: 28
登録日時: 3年前
住所: アンドロメダ銀河系のどこかに住んでいる

Re: (コードレビュー)(url解析機)[雑談]

#8

投稿記事 by ぷぷぷジューシー » 3年前

バグ探しで設計の3と4の間に<code>printf("temppath:\t %s\n", rhost);
printf("temphost:\t %s\n", rpath); </code>を入れたですがp6やpr/などが表示されていてこれを何を意味するのか分かりません。

<code>
#define __STDC_WANT_LIB_EXT1__ 1
#include <stdio.h>
#include <string.h>
#include <uchar.h>
#include <limits.h>
#include <stdlib.h>


int geturlparserfunc(const char*, char*, char*);

int main(void)
{
char hostname, pathname;
geturlparserfunc("http://goo/com/", &hostname, &pathname);

printf("ホスト名; %s\n", hostname);
printf("パッチ名: %s\n", pathname);

}

int geturlparserfunc(const char* name, char* rhost, char* rpath)
{
size_t bmfunc_rlen;
int inputlen = strlen(name) + 2;
char input[(int)inputlen];
char32_t inp32b[(int)inputlen];
int arraycount1 = 0, arraycount2 = 0;
mbstate_t mbstate = 0;

int slashconunt = 0, colon = 0;
char temppath[MB_LEN_MAX];
char temphost[MB_LEN_MAX];


strcpy_s(input, inputlen, (char*)(name));

while (input[arraycount2] != '\0'){
bmfunc_rlen = mbrtoc32(&inp32b[arraycount1], &input[arraycount2],MB_LEN_MAX * 5, &mbstate);
if ( (size_t)(-1) == bmfunc_rlen ){
printf("エラーが発生しました\n");
printf("エンコードエラー又はバイト数が足りませんの可能性があります。");
printf("エラー size_t サイズ : %zu\n", bmfunc_rlen);
break;
} else if ( (size_t)(-2) == bmfunc_rlen ){
printf("エラーが発生しました\n");
printf("マルチバイト文字の可能性があります。");
printf("エラー size_t サイズ : %zu\n", bmfunc_rlen);
break;
} else if ( (size_t)(-3) == bmfunc_rlen ){
arraycount1++;
continue;
} else if (( (size_t)(0) == bmfunc_rlen ) ){
break;
} else if ( (size_t)(0) < bmfunc_rlen ){
arraycount1++;
arraycount2 += (int)(bmfunc_rlen);
}
}

int n = 0;
while (arraycount1 > n){
n++;
if (inp32b[n] == ':' || inp32b[n] == '/'){
if (inp32b[n] == ':'){
colon = 1;
}
if (inp32b[n] == '/'){
slashconunt += 1;
}
}
if (slashconunt > 3 && colon == 1) {
bmfunc_rlen = c32rtomb(temppath ,inp32b[n], &mbstate);
if ((size_t)(-1) == bmfunc_rlen){
printf("エラーが発生しました\n");
printf("有効なワイド文字ではありません\n");
printf("エラー size_t サイズ : %zu\n", bmfunc_rlen);
break;
}
strcat(rpath, temppath);
}

if (slashconunt == 2 && colon == 1){
// rhost 処理
bmfunc_rlen = c32rtomb(temphost ,inp32b[n], &mbstate);
if ((size_t)(-1) == bmfunc_rlen){
printf("エラーが発生しました\n");
printf("有効なワイド文字ではありません\n");
printf("エラー size_t サイズ : %zu\n", bmfunc_rlen);
break;
}
strcat(rhost, temphost);
}
}

printf("temppath:\t %s\n", rhost);
printf("temphost:\t %s\n", rpath);
rhost[sizeof(rhost) - 1] = '\0';
rpath[sizeof(rpath) - 1] = '\0';

arraycount1 = 0, arraycount2 = 0, slashconunt = 0, colon = 0 , bmfunc_rlen = 0;

return 0;
}
</code>
面倒なことはCGo使おう!

返信

“C言語何でも質問掲示板” へ戻る