プログラマ専用SNS ミクプラ

コード:

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

 struct prototype{
  int rank;
  char *str;
 };
 
 typedef struct prototype test;
 
 #define SIZE 256
 
 static int st_lim=0;
 static int st_used=0;
 static char *st_buffer=NULL;
 int st_continue=1;
 int lim_str=0;
 
 void read_char(int ch)
 {
  if(st_used==st_lim)
  {
   st_buffer=realloc(st_buffer,(st_lim+SIZE)*sizeof(char*));
   st_lim+=SIZE;
  }
  st_buffer[st_used++]=ch;
 }
 
 void read_line(char **str)
 {
  int ch=0,i,j;
  
  for(i=0;ch!=EOF&&ch!='\n';i++)
  {
   st_used=0;
   for(j=0;;j++)
   {
    ch=getchar();
    if(!('0'<=ch&&ch<='9')&&ch!='\n')exit(1);
    
    if(ch=='\n'||ch==EOF)
    {
     if(j==0||ch==EOF)st_continue=0;
     read_char('\0');
     break;
    }
    read_char(ch);
   }
   printf("#%s",st_buffer);
  //strcpy(*str,st_buffer);
  printf("==%s#\n",*str);
  }
 }
 
 
 int main(void)
 {
  test *data,data_ad;
  data=&data_ad;
  
  read_line(&data->str);
  /*char *str_n;
  char str_ad;
  str_n=&str_ad; 
  
  read_line(&str_n);
  printf("%s",str_n);*/
  return 0;
 }

見にくく醜い部分が多々あると思いますがご了承ください
これまでに作ってきた関数を、今度は構造体でも使えるかな、と作成したものを構造体に使ってみたところ（？）
65~70行目の見慣れたものでならエラー無く実行できたため,構造体でもアクセスの仕方?表記の仕方?が少し違うだけだろうからいけるんだろうなぁ,とおもっていたので
59~62行目を実行したところ51行目のstrcpy関数の部分でエラーが出たらしく実行できませんでした.
なので適当な文字列をdata->strに与えたほうがよろしいかなと,65~70行目では不必要であった文字列を　data->str="abc";のように61行目に挿入したところうまいこといきました

ここで,なぜ65~70行目,59~62行目にこのような差(文字列を与える必要のあるものとないもの)が出てしまったのかが疑問で仕方ありません.ご存知の方ご教授お願いいたします

オフトピック

orz
ユーザーネームとトピックネームが逆です.

文字列についてさんが書きました:65~70行目の見慣れたものでならエラー無く実行できたため,構造体でもアクセスの仕方?表記の仕方?が少し違うだけだろうからいけるんだろうなぁ,とおもっていたので

str_adには1文字分の領域しか無いので、終端のナル文字を含めて2文字以上の文字列をコピーすると範囲外アクセスで未定義動作になります。
そのようなことはしてはいけません。
たまたまエラーが出ず、悪いことをしているのに気づかなかったあなたは運が悪いです。

文字列についてさんが書きました:59~62行目を実行したところ51行目のstrcpy関数の部分でエラーが出たらしく実行できませんでした.

初期化されていない自動変数の値は不定であり、あなたはその不定の値を用いることで未定義動作を起こしました。

文字列についてさんが書きました:65~70行目では不必要であった文字列を　data->str="abc";のように61行目に挿入したところうまいこといきました

再び悪運ですね。
文字列リテラルを書き換えようとすると未定義動作を起こします。

文字列についてさんが書きました:ここで,なぜ65~70行目,59~62行目にこのような差(文字列を与える必要のあるものとないもの)が出てしまったのかが疑問で仕方ありません.

そもそも、どうして文字列を与える必要のあるものとないものがあると思ったのですか？
(どうせ間違っていますが)「見慣れたもの」と同様にchar str_ad; data->str = &str_ad;とする実験をしなかったのですか？
もしくは、しても運良くエラーになったのですか？

というわけで、正しい処理は「十分な領域を確保し、そこに文字列をコピーする」です。
それは、例えばこのようになるでしょう。

コード:

void read_line(char **str)
{
 int ch=0,i,j;
 
 for(i=0;ch!=EOF&&ch!='\n';i++)
 {
  st_used=0;
  for(j=0;;j++)
  {
   ch=getchar();
   if(!('0'<=ch&&ch<='9')&&ch!='\n')exit(1);
   
   if(ch=='\n'||ch==EOF)
   {
    if(j==0||ch==EOF)st_continue=0;
    read_char('\0');
    break;
   }
   read_char(ch);
  }
  printf("#%s",st_buffer);
  *str=malloc(strlen(st_buffer)+1); /* 領域を確保 */
  if(*str==NULL)exit(2); /* 確保できたかチェック */
  strcpy(*str,st_buffer); /* 文字列をコピー */
  printf("==%s#\n",*str);
 }
}

オフトピック

とりあえず元のコードを尊重しましたが、
どうせch!=EOF&&ch!='\n'は最初はch=0なので真、次は内側のループを抜ける条件ch=='\n'||ch==EOFより偽なので、
外側のループは意味ないですよね…。
ローカル変数iもこのfor文以外に使われていないようですし。

＞質問者さん
とりあえず、最初に行なうことは、行ないたいことを日本語で書いてみることではないかと思います。
これができれば、プログラミング言語に置き換えることができるかもしれません。
「かもしれません」というのは、日本語で書けたからといって、100％プログラミング言語に置き換えることができるとは限らないという意味です。
正しく置き換えられないとき、それをバグといいます。
一方、行ないたいことを日本語で書くことができない場合、プログラミング言語に置き換えることはおそらくできないでしょう。

なんと...
自分のせいでもありますがコンパイラの設定を変えるかコンパイラ事態を別のものに変えたほうがいいですかね?

また、文字列を変えるには配列を扱うしかないということですか??

おっしゃるとおりかもしれません
当初の予定,目標を忘れかけていました

構造体のメンバとしての文字列をキーボードで入力することができればいいなと思っています

コード:

 
 #include<stdio.h>
 #include<stdlib.h>
 #include<string.h>
 
 int main(void)
 {
  char *str;
  char buf[256];
  
  scanf("%s",buf);
  
  str=malloc(sizeof(str)*strlen(buf)+1);
  strcpy(str,buf);
  printf("%s",str);
  
  return 0;
 }

こういった手順を踏むのが一番綺麗?規則に忠実?な方法ですか??

おーばーさんが書きました:
コード:
 
 #include<stdio.h>
 #include<stdlib.h>
 #include<string.h>
 
 int main(void)
 {
  char *str;
  char buf[256];
  
  scanf("%s",buf);
  
  str=malloc(sizeof(str)*strlen(buf)+1);
  strcpy(str,buf);
  printf("%s",str);
  
  return 0;
 }
こういった手順を踏むのが一番綺麗?規則に忠実?な方法ですか??

いいえ。
少なくとも、一番綺麗でも一番規則に忠実でもないでしょう。
まず、「規則に忠実」に近くするためには、

scanfの戻り値で読み込めたかを確認する
バッファオーバーフローを起こさないようにscanfで読み込む最大サイズを指定する
mallocの戻り値を見て成功したことを確認する
(mallocで確保した領域をfreeで開放する) : どうせ終了時にはOSが開放してくれるのでこの場合は必要ないという考え方もあります

という修正をするといいでしょう。

さらに、char型の配列を確保したいのに、なぜかchar*型のサイズを掛けている上、最後に中途半端に1を足しているのが良くないです。
sizeof(char)は1と定義され、sizeof(str)は1以上のはずなので確保が成功すれば範囲外アクセスにはならず、実行上問題は無いはずですが、綺麗ではありません。

これらの問題を修正すると、例えばこうなるでしょう。

コード:

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

int main(void)
{
 char *str;
 char buf[256];

 if (scanf("%255s", buf) != 1)
 {
  fputs("read error\n", stderr);
  return 1;
 }

 str = malloc(sizeof(char) * (strlen(buf) + 1));
 if (str == NULL)
 {
  perror("malloc");
  return 1;
 }
 strcpy(str, buf);
 printf("%s", str);

 free(str);
 return 0;
}

さらに、このサンプルなら無駄に領域を確保してstrcpy()なんてめんどくさいことをせずに、読み取った文字列をそのまま直接出力すればいいでしょう。

コード:

#include<stdio.h>

int main(void)
{
 char buf[256];

 if (scanf("%255s", buf) != 1)
 {
  fputs("read error\n", stderr);
  return 1;
 }

 printf("%s", buf);

 return 0;
}

おーばーさんが書きました:自分のせいでもありますがコンパイラの設定を変えるかコンパイラ事態を別のものに変えたほうがいいですかね?

No: 1のコードをGCCとclangでコンパイルしても全く警告が出なかったので、その必要性は薄いでしょう。

おーばーさんが書きました:また、文字列を変えるには配列を扱うしかないということですか??

C言語では文字列はヌル文字で終端された文字の配列として表わされるので、文字列を扱うなら配列を扱うしかないでしょう。

おーばーさんが書きました:構造体のメンバとしての文字列をキーボードで入力することができればいいなと思っています

なるほど。
では、次にそれを実現するための具体的な手順を日本語で説明してみてください。

無事お二方の助言によりプログラムを学ぶ途中ででた本の例題の改良版のようなものが自分の思うように動作しました！

言葉で言うとユーザーに数値を打ち込んでもらいその数を比較する、といったものなのですが入力部分は別のものを作る際にも少しの修正でなんとか使いまわすこともできそうです

コード:

 /*read_str.h*/

 #ifndef READ_STR
 #define READ_STR
 
 #include<stdio.h>
 
 extern int lim_str;
 extern int line_num;

 struct prototype{
  int rank;
  char *str;
 };
 
 #endif

コード:

 /*read_str.c*/

  #include<stdio.h>
 #include<string.h>
 #include<stdlib.h>
 #include"read_str.h"
 
 #define SWAP(type,no1, no2) {type temp=no1;no1=no2;no2=temp;}
 
 #define SIZE 256
 typedef struct prototype test;
 
 static int st_lim=0;
 static int st_used=0;
 static int struct_lim=0;
 static char *st_buffer;
 static int struct_used=0;
 extern int st_continue=1;
 int lim_str=0;
 int line_num=0;
 
 void read_char(int ch)
 {
  if(st_used==st_lim)
  {
   st_buffer=realloc(st_buffer,(st_lim+SIZE)*sizeof(char));
   if(st_buffer==NULL)exit(2);
   st_lim+=SIZE;
  }
  st_buffer[st_used]=ch;
  st_used++;
 }
 
void read_line(char **str)
{
 
 int ch=0,i,j;
 //for(i=0;ch!=EOF&&ch!='\n';i++)
 //{
  st_used=0;
  for(j=0;;j++)
  {
   ch=getchar();
   if(!('0'<=ch&&ch<='9')&&ch!='\n')exit(1);
   
   if(ch=='\n'||ch==EOF)
   {
    if(j==0||ch==EOF)st_continue=0;
    read_char('\0');
    break;
   }
   read_char(ch);
  }
  *str=malloc(sizeof(char)*strlen(st_buffer)+1);/* 領域を確保 */
  if(*str==NULL)exit(2); /* 確保できたかチェック */
  strcpy(*str,st_buffer); /* 文字列をコピー */
 //}
  line_num++;
 }
 void read_struct_str(test *data)
 {
  int i,j;
  char *str;
  for(;st_continue!=0;struct_used++){
   if(struct_used==struct_lim)
   {
    data=realloc(data,sizeof(test)*(struct_lim+SIZE));
    if(data==NULL)exit(2);
    struct_lim+=SIZE;
    memset(data,0,sizeof(test*)*struct_lim);
   }
   read_line(&str);
   data[struct_used].str=malloc(sizeof(test*)*strlen(st_buffer)+1);
   if(data[struct_used].str==NULL)exit(2);
   strcpy(data[struct_used].str,str);
  }
 }
 
 void sort(test *data)
 {
  int i,j;
  for(i=1;i<line_num-1;i++)
  {
   for(j=i-1;0<=j;j--)
   {
    if(data[j].rank<=data[j+1].rank)
    {
     j=0;
    }
    else {
     SWAP(test,data[j+1],data[j])
    }
   }
  }
 }

 void ranking(test *data)
 {
  int i,j;
  
  for(i=0;i<line_num-1;i++)
  {
   for(j=i+1;j<line_num-1;j++)
   {
    int temp1,temp2;
    if(i==j)continue;
    temp1=atoi(data[i].str);
    temp2=atoi(data[j].str);
    if(temp1<temp2)data[j].rank++;
    else if(temp1==temp2);
    else data[i].rank++;
   }
   data[i].rank=(line_num-1)-data[i].rank;
  }
  sort(data);
 }

コード:

 
/*main.c*/

#include<stdio.h>
 #include<string.h>
 #include<stdlib.h>
 #include"read_str.h"
 
 #define SWAP(type,no1, no2) {type temp=no1;no1=no2;no2=temp;}
 
 typedef struct prototype test;
 
 int main(void)
 {
  int i,j;
  test *data;
  
  data=malloc(sizeof(test)*256);
  if(data==NULL)exit(2);
  
  read_struct_str(data);
  ranking(data);  
  if(line_num==2+1)
   printf("high %s low %s\n",data[1].str,data[0].str);
  
  else
    for(i=0;i<line_num-1;i++)printf("%d位 ( %s )\n",
                          /*line_num-*/data[i].rank/*-1*/,
                                       data[i].str);

  return 0;
 }

このようなものです,動作はしましたが,もっとこうしたほうがいい、そもそもこの部分、動作したからいいもののおかしいよ、　ここ、この前指摘したのにそのままなんだね、、
等ございましたらお教えください。

あえて言うならば、字下げが1文字分というのはいかにも見づらいなぁ、と思います。
まあこのあたりは宗教論争になりそうですので深く突っ込まないことにしますが、
C言語ならば4文字分あたりが適当かと個人的には思います。

なるほどです.次質問するときは字下げについて調べ,独自のスタイルを修正し以前よりも見やすいプログラムを作って見せます！！

おーばーさんが書きました:このようなものです,動作はしましたが,もっとこうしたほうがいい、そもそもこの部分、動作したからいいもののおかしいよ、　ここ、この前指摘したのにそのままなんだね、、
等ございましたらお教えください。

とりあえずざっと見ただけでわかったことを書きます。

まず致命的な問題として、67行目においてread_struct_str関数でreallocした結果の新しいバッファへのポインタを呼び出し元に返していないので、
入力の行数が十分多い場合に解放された領域にアクセスしてしまう可能性があります。
どうせ呼び出し元で最初から十分な要素数を確保する方針なら、reallocなんて使うのをやめて、引数で読み込める最大の要素数を渡してそれに従うようにするといいでしょう。

次に、70行目においてdataに指されている配列の要素はtestなのに、誤って要素の型をtest*として計算してしまっているので、
普通は配列全体がゼロクリアされないでしょう。クリアされていない部分を読み込まず、
かつこのmemsetで範囲外アクセスを起こすような特殊な環境(sizeof(test) < sizeof(test*)となる環境、その必要条件としてポインタの指す先の型によってポインタのサイズが違うことがある環境)
で実行しないのであれば無害でしょうが、気持ち悪いです。
そして、rankメンバを明示的に初期化していないので、要素数によってはmallocで確保した未初期化の領域をranking関数で読み込んで未定義動作になるかもしれません。

そして、73行目において、明らかにサイズ計算がおかしいです。
char型を要素とする配列を確保するはずなのになぜか文字列の長さにsizeof(test*)をかけていますし、最後になぜか中途半端に1を足していて整合性がとれていません。
この前指摘したのにそのままどころか、全然配列に関係ない型(のサイズ)を計算に使っているという点でさらにひどくなっていますね。
read_line関数が正常に働けば同じ内容になるとはいえ、strに入力を取り、st_bufferに入っている文字列の長さを用いてメモリを確保し、またstrからデータをコピーするという一貫性のなさも気持ち悪いです。
さらにstrを開放せずメモリリークが発生しています。
こんな無駄なメモリ確保とコピーをせず、取得したstrを直接data[struct_used].strに代入すればいいでしょう。

また、54行目において、+1だけ掛け算をせずに独立して足しているのが気になります。
sizeof(char)は1と定義されている(N1570 6.5.3.4の4)ので無害ですが、
いっそsizeof(char)を取って malloc(strlen(st_buffer)+1);
もしくはchar型を要素とする配列を確保することを強調するなら malloc(sizeof(char)*(strlen(st_buffer)+1));
とするといいでしょう。

厳しいご指摘大変助かります！

read_struct_str関数の回りくどさには全くその通りですね...
また,なんども動的確保においてのサイズの計算時の式の間違いの指摘もありがたいです,理解できていない部分がありそうで,次の自分の課題なのかなとも気づかせていただけました！

プログラマ専用SNS ミクプラ

おーばー

おーばー

Re: おーばー

Re: おーばー

Re: おーばー

Re: おーばー

Re: おーばー

Re: おーばー

Re: おーばー

Re: おーばー

Re: おーばー

Re: おーばー

Re: おーばー

Re: おーばー

Re: おーばー