質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

ただいまの
回答率

89.99%

文字列をバッファに格納するプログラム

解決済

回答 3

投稿 編集

  • 評価
  • クリップ 0
  • VIEW 395

aiueo12345

score 19

前提・実現したいこと

バッファに文字列を格納するプログラムをCで書きました。
以下にそのプログラムを示します。

#include <stdio.h>

#define BUFSIZE 24

struct buffer {
  char store[BUFSIZE];
  int head , tail;
};

int put_str(struct buffer *buf, char *str) {
  int i = buf->tail;

  while (i - buf->head < BUFSIZE) {
    buf->store[i++ % BUFSIZE] = *str;
    if (*str == '\0') {
      buf->tail = i;
      return 1;
    } else {
      str++;
    }
  }

  return 0;
}

int get_str(struct buffer *buf, char *dest) {
  int i = buf->head;

  if (i == buf->tail/BUFSIZE ) return 0;

  do {
    *dest = buf->store[i] ;
  } while ( buf->store[i++] != '\0');
  buf->head = i;
  return 1;
}


int main() {
  struct buffer buf;
  char dest;
  int i;

  buf.head = buf.tail = 0;
  for(i = 0; i < BUFSIZE; i++)
    buf.store[i] = '\0';

  put_str(&buf, "ten");
  put_str(&buf, "six");
  put_str(&buf, "three");
  put_str(&buf, "four");

  get_str(&buf, &dest);

  while(dest != '\0')
    printf("%c", dest++);
  puts("");

  return 0;
}

格納文字列はバッファ領域を超えないとします。

発生している問題

main関数内で文字列を表示させようと試みたところ、上手くいきませんでした。
コンパイルし実行することはできたのですが、何も表示されませんでした。

おそらくmain関数内だけを変更すれば上手くいくと思います。

get_str関数に与える二つ目の引数と、main関数内のprintfに問題がありそうな気がするのですが、どうでしょうか。

get_strの二つ目の引数としてchar型の配列の先頭アドレスを渡してみたり、char型のポインタを渡してみたりしたのですが、上手くいきませんでした。

どのように改善すればよいでしょうか?

  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

回答 3

+3

もうすべてがぶっ壊れているといって過言ではありません。

  while (i - buf->head < BUFSIZE) {
    buf->store[i++ % BUFSIZE] = *str;
    if (*str == '\0') {
      buf->tail = i;
      return 1;
    } else {
      str++;
    }
  }

文字列のコピーを自分でループで書くのは完全に誤った発想です。文字列のコピーは常にmemcpy関数によって行われるべきです。短く、高速に、安全に書けます。Don't Repeat Yourself!ちなみにstrcpyとかsprintfとかstrcatとかは安全ではありませんから使うべきではありません。

i++ % BUFSIZEというのは謎で、これだけ見るとあたかもリングバッファのように見えてくるのですが、他の部分のコードから見るにそんなことはないただのキュー構造ですから、剰余を取るのは意味不明です。buffer overrun対策は文字列のコピーの前にバッファの残りの容量とコピーしようとする文字列の長さから計算して弾くべきです。

int get_str(struct buffer *buf, char *dest)

はい、おかしいですね。文字列を引数dest経由で返したいと言うのが意図のはずですが、ここで関数が呼び出されたときにdestが指し示している領域の大きさをget_strが知る手段がありません。この時点でぶっ壊れています。引数を足して大きさも渡しましょう。

  do {
    *dest = buf->store[i] ;
  } while ( buf->store[i++] != '\0');

そもそもdestを動かしていないのでどんなにループが回ろうとも同じ場所に書き込みます。ぶっ壊れていますね!

というかそもそも文字列のコピーをループで書こうとするのは誤りです。予め長さの検査をした上でmemcpyを使いましょう。自分でループ書くより短く、高速に、安全に書けます。Don't Repeat Yourself!

int main() {
  struct buffer buf;
  char dest;
  //中略
  get_str(&buf, &dest);

ここで変数destの型に注目しましょう。まさかのchar型です。一体どうやって文字列を格納するんでしょうね?実にぶっ壊れています。適当な大きさのメモリー領域を確保する必要がありますね(配列でもいいしmallocのようななにかでもいい)

  while(dest != '\0')
    printf("%c", dest++);

文字列を出力するのに、わざわざ一文字ずつ表示してループを回すというじつにクレイジーなコードです。素直に%sを使って出力しましょう。


というわけで面白いくらいぶっ壊れまくっています。疲れたので言及しませんが、きっとこれ以外にもぶっ壊れているところがあるでしょう。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2019/06/20 22:46

    この問題は筑波大学コンピュータサイエンス専攻の院試の過去問です。
    (ちなみに私が院試を受けるのは数年後です)
    他の問題も解いていて、なかなか意地の悪い問題があるように感じています。

    strcpyについて、そういうことだったんですね。
    memcpyでは引数として確保しているメモリを明示するという点で安全性が高いのですね。

    strcpyの方が使い慣れていますがmemcpyを使うようにしていきたいと思います。

    キャンセル

  • 2019/06/21 10:33

    実際C標準関数の安全性というかbuffer overrunの可能性にめちゃくちゃうるさいMSVCはmemcpy以外の大抵の文字操作系関数について警告を出してくるのですが、実際安全に使えないor他の関数のほうが効率的なので仕方ない。
    C11でMSVC独自拡張だった_sが末尾につく関数群がoptionalな形で標準入りしたのですが、結局_sをつけるんじゃなくてmemcpyしないとだめだよね、みたいな話だと思うんですがやっぱ取り除こうなんて話もあるので・・・。

    筑波大学・・・、ひええぇ。

    キャンセル

  • 2019/06/21 18:53

    memcpy以外には警告を出してくる環境もあるのですね。
    詳しいことはよく分かりませんが、こういった話があると知れたのはいい経験になりました。
    ありがとうございます。

    キャンセル

checkベストアンサー

+1

問題点1
get_str関数で返したいのは文字列(charの配列)だが、関数に渡しているのはchar型の変数である。main関数内部のdestを受け取り用の配列にする必要がある。

問題点2
get_str関数が文字列を返す仕様になっていない。
ポインタを更新せず、同じ領域を上書きしている。

問題点3
get_str内部の「割り算」の意味は?

追記:私が書く場合

int put_str(struct buffer *buf, const char *str) {
  int i, nFree = BUFSIZE - (buf->tail - buf->head);
  if(nFree == 0) return 0;
  for(nFree--; nFree > 0 && *str; nFree--){
    buf->store[buf->tail++ % BUFSIZE] = *str++;
  }
  buf->store[buf->tail++ % BUFSIZE] = '\0';
  return !*str;
}

int get_str(struct buffer *buf, char *dest, int size) {
  if(buf->head == buf->tail) return 0;
  if(size == 0) return 0;
  for(size--; size > 0 && buf->store[buf->head % BUFSIZE]; size--){
    *dest++ = buf->store[buf->head++ % BUFSIZE];
  }
  *dest = '\0';
  if(buf->store[buf->head % BUFSIZE]) return 0;
  buf->head++;
  return 1;
}


int main() {
  struct buffer buf;
  char dest[BUFSIZE];
  int i;

  buf.head = buf.tail = 0;
  for(i = 0; i < BUFSIZE; i++){
    buf.store[i] = '\0';
  }

  put_str(&buf, "ten");
  put_str(&buf, "six");
  put_str(&buf, "three");
  put_str(&buf, "four");

  get_str(&buf, &dest, sizeof(dest) / sizeof(dest[0]));
  puts(dest);

  return 0;
}


携帯から打っているのでミスがあるかもしれませんが、こんな感じです。

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2019/06/21 00:51

    たしかにおっしゃる通りでした、すみません。
    例として出していただいたものは、最初にBUFSIZE文字の文字列を保存した場合だと思いますが、そうであれば納得できました。

    丁寧に教えてくださりありがとうございました。

    最後に一つ、質問の仕方についてご教授いただけますでしょうか。
    今後もテラテイルでこのような問題を質問しようかと思うのですが、『正解を教えてください』や『自分の解答はこうこうなのですが合っていますか?』というのは無責任だと思います。
    ですが、今回のように色々混乱させてしまうこともあると思うのですが、どのような質問が適しているでしょうか?
    特に、自分で解答できれば実行してみて確かめられますが、わからない部分があった場合は難しいです。
    そもそも、テラテイルでそのような質問をするのが間違いでしょうか?

    キャンセル

  • 2019/06/21 07:04

    大学院の過去問を解いてみたが、自分の解答ではなぜ上手く動かないのか理解できない、自分はこう考えてこの解答にしたが、どのように考える必要があるのか、ということを聞くのがよいと思います。特に、考え方・理由を言語化する過程は自らを客観的に分析し直すために有効です。

    キャンセル

  • 2019/06/21 08:37

    自分の答えだけでなく、思考も示すことが大切なのですね。

    本当に色々とありがとうございました。

    キャンセル

+1

回答ではないです。すみません。
このプログラムって要は、memsetやstrcat、printfでパパッとできることを自力でやってみた、もしくはやってみたいという主旨でよいのでしょうか。

#include <stdio.h>
#include <string.h>

int main(void)
{
    char store[BUFSIZE];

    memset(store, 0x00, sizeof(store));

    strcat(store, "ten");
    strcat(store, "six");
    strcat(store, "three");
    strcat(store, "four");

    printf("%s", store);

    return 0;
}

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2019/06/20 21:18

    混乱を招いてしまい申し訳ないです。
    私としてはmajiponiさんにおっしゃっていただいたとおり、格納した文字列一つずつを取り出したいと考えております。

    キャンセル

  • 2019/06/20 21:21

    なるほど。
    ということはやはりNULL区切りで文字列を格納したい、ということでしょうか。
    そういった具体的な仕様を提示しないまま、できない、わからない、ということを質問しないほうがよいですよ。

    キャンセル

  • 2019/06/20 21:24

    おっしゃる通り、ナル文字で区切るということです。
    質問に不備が多く、すみませんでした。

    キャンセル

15分調べてもわからないことは、teratailで質問しよう!

  • ただいまの回答率 89.99%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる