21235


環境はViausl Studio 2015 communityです。

以下のようなファイルと、

[code]

        #include "intRect.hpp"
    
        int main()
        {
          std::cout << "declare..." << std::endl;
          intRect rectangle;
    
          rectangle.setSize(10, 5);
    
    
          std::cout << "set value..." << std::endl;
          // set value
          for (int x = 0; x < 10; x++)
          {
            for (int y = 0; y < 5; y++)
            {
              rectangle.setValue(x, y, x + y);
            }
          }
    
          std::cout << "printing..." << std::endl;
          // print value
          for (int x = 0; x < 10; x++)
          {
            for (int y = 0; y < 5; y++)
            {
              rectangle.printValue(x, y);
            }
          }
          return 0;
    
        }
    
[/code]

以下のようなインクルードファイルがあるのですが、

[code]

        #ifndef INTRECT_HPP
        #define INTRECT_HPP
    
        #include <memory>
        #include <iostream>
    
        struct intRect
        {
          int width = 0;
          int height = 0;
          int size = 0;
    
          //std::unique_ptr<int[]> intrect_body = nullptr;
          std::unique_ptr<int> intrect_body;
    
          void setSize(const int arg_width, const int arg_height)
          {
            width = arg_width;
            height = arg_height;
    
            intrect_body.reset(new int(width * height));
          }
    
          void setValue(const int& arg_x, const int& arg_y, const int& arg_value)
          {
            if (width == 0 || height == 0)
            {
              return;
            }
    
            int position = arg_y * width + arg_x;
    
            if (position > width * height)
            {
              return;
            }
    
            intrect_body.get()[position] = arg_value;
          }
    
          void printValue(const int& arg_x, const int& arg_y)
          {
    
            if (width == 0 || height == 0)
            {
              return;
            }
    
            int position = arg_y * width + arg_x;
    
    
            if (position > width * height)
            {
              return;
            }
    
            std::cout << "value at position (" << arg_x << ", " << arg_y << ") is " << intrect_body.get()[position] << std::endl;
    
          }
    
    
        };
    
    
        #endif 
    
[/code]

以下の画像のように、止まるところがおかしなところで、勝手にリソースが解放されてしまっているようにも見えるのですが、原因がわかりません。  
アウトプットには  
Exception thrown at 0x0F6FB4E4 (ucrtbased.dll) in smapowraptest.exe:
0xC0000005: Access violation reading location 0x000003B2.  
と表示されています。  
Exception thrown at 0x775A324F (ntdll.dll) in smapowraptest.exe: 0xC0000005:
Access violation reading location 0x00000009.  
と表示されることもあり、挙動がよくわからない状態になります。

MSYS2でg++でコンパイルして流すと、正常に動くのですが、何か間違っているのでしょうか。

[![画像の説明をここに入力](https://i.stack.imgur.com/wDBPR.png)](https://i.stack.imgur.com/wDBPR.png)


CC BY-SA 3.0
2016-01-23T13:40:16.567
21231
2016-01-23T21:20:04.853
5835
question
c++ visual-studio
visual studioで、unique_ptrを使っているときに予期せぬところでメモリアクセス違反が起こる


_MSYS2でg++でコンパイルして流すと、正常に動くのですが、何か間違っているのでしょうか。_  
g++でコンパイルして動くのはメモリ割当ルールの違いなどのせいで「たまたま」動いているように見えるだけだと思われます。最近C++のコードは書いていないのですが、そういう人間から見ても、あなたのコードはかなり無茶苦茶ですね。

[code]

    std::unique_ptr<int[]> intrect_body = nullptr;
    
[/code]

複数のint型の値を保持しないといけないのに、なぜ配列の方をコメントアウトしてしまっているのでしょう。`[]`付きが正解です。

[code]

        intrect_body.reset(new int(width * height));
    
[/code]

コメントにもありますが、これはintを1つ分確保して、その値をwidth * heightにしているだけです。width *
height個分のintを確保するなら、

[code]

        intrect_body.reset(new int[width * height]);
    
[/code]

と書かないといけません。

[code]

        intrect_body.get()[position] = arg_value;
    
[/code]

これは趣味の問題で、絶対ダメというわけではないのですが、unique_ptrのoperator[]はうまく定義されていて、要素をアクセスするだけなら、get()は要りません。

[code]

        intrect_body[position] = arg_value;
    
[/code]

(もう一箇所のget()も同様。)

上記を全部直してもまだ何かしらエラーが出るのならコメント等でご連絡ください。


CC BY-SA 3.0
2016-01-23T14:06:48.697
21233
2016-01-23T14:06:48.697
13972
21231
answer


`std::unique_ptr<int>`は`int`へのポインターを保持するものです。`int[]`配列を保持するのであればコメントアウトされているように`std::unique_ptr<int[]>`を使用する必要があります。

また`new int(width * height)`はintを格納するメモリを確保した上でintの値を`width *
height`で初期化します。`new int[width *
height]`とすべきですが、それ以上に、（スマートポインターを使用している以上、自動的に解放されるものの）ソースコード上の対称性として`new`しておいて`delete`しないというのも不安にかられます。これについては`new`による確保とコンストラクターの呼び出し、その上で`std::unique_ptr`の構築までを一気に済ませてくれる`std::make_unique()`が用意されています。

`setSize()`する前に`setValue()`
`printValue()`を呼ばれると困るため事前チェックをされているようですが、であれば`setSize()`はコンストラクターとすべきです。コンストラクターであればほぼ確実に呼ばれますからチェックも必要なくなります。

事前チェック自体も、異常状態・不正値を無視して`return`してしまうのも無責任です。何らかの`assert`を行うことをお勧めします。Visual
C++ではデバッグビルドではエラーレポートしリリースビルドではコードが削除される[_ASSERTE](https://msdn.microsoft.com/ja-
jp/library/ezb1wyez.aspx)マクロが用意されています。

以上を総合すると

[code]

    struct intRect {
        int width;
        int height;
        std::unique_ptr<int[]> intrect_body;
    
        intRect(int arg_width, int arg_height) : width(arg_width), height(arg_height), intrect_body(std::make_unique<int[]>(width * height)) {}
    
        void setValue(int arg_x, int arg_y, const int& arg_value) {
            _ASSERTE(arg_x < width && arg_y < height);
            intrect_body[arg_y * width + arg_x] = arg_value;
        }
    
        void printValue(const int& arg_x, const int& arg_y) {
            _ASSERTE(arg_x < width && arg_y < height);
            std::cout << "value at position (" << arg_x << ", " << arg_y << ") is " << intrect_body[arg_y * width + arg_x] << std::endl;
        }
    };
    
[/code]

と書けます。


CC BY-SA 3.0
2016-01-23T14:10:48.973
21235
2016-01-23T21:20:04.853
2016-01-23T21:20:04.853
4236
4236
21231
answer