6

私の C++ の経験不足、またはガベージ コレクション言語での初期の学習は、現時点では本当に私を悩ませており、C++ で文字列を操作する際に問題があります。

非常に明確にするために、 std::string または equlivents を使用することはオプションではありません-これはずっと char* です。

つまり、私がしなければならないことは非常に単純で、基本的には文字列を連結することです。実行時には、2 つのクラスがあります。

1 つのクラスには、ベース ファイル名の形式で「タイプ」情報が含まれます。

ヘッダーで:

char* mBaseName;

その後、.cpp では、他の場所から渡された情報が読み込まれます。

mBaseName = attributes->BaseName;

2 番目のクラスは、ベース ファイル名のサフィックスの形式でバージョン情報を提供します。これは静的クラスであり、現在次のように実装されています。

static const char* const suffixes[] = {"Version1", "Version", "Version3"}; //etc.

static char* GetSuffix()
{
    int i = 0;
    //perform checks on some data structures
    i = somevalue;
   return suffixes[i];
}

次に、実行時に基本クラスが必要なファイル名を作成します。

void LoadStuff()
{
    char* suffix = GetSuffix();
    char* nameToUse = new char[50];
    sprintf(nameToUse, "%s%s",mBaseName,suffix);

    LoadAndSetupData(nameToUse);
}

そして、問題はすぐにわかります。nameToUse は削除されず、メモリ リークが発生します。

サフィックスは固定リストですが、ベースファイル名は任意です。作成された名前は、「LoadStuff()」の終わりを超えて存続する必要があります。これは、いつ、どのように使用されるかが明確でないためです。

私が心配しすぎているか、非常に愚かなのかもしれませんが、LoadStuff() と同様のコードが他の場所でも発生するため、解決する必要があります。安全で「ハッキングされていない」ソリューションを確認するために物事がどのように機能するかについて十分に知らないので、イライラします. C# では、次のように記述します。

LoadAndSetupData(mBaseName + GetSuffix());

心配する必要はありません。

コメント、提案、またはアドバイスをいただければ幸いです。

アップデート

LoadAndSetupData() を呼び出しているコードの問題は、ある時点でおそらくファイル名をコピーしてローカルに保持することですが、実際のインスタンス化は非同期であり、LoadAndSetupData は実際に物事をキューに入れ、その時点で少なくとも、渡された文字列がまだ存在することが期待されます。

私はこのコードを制御していないため、機能を更新できません。

4

13 に答える 13

3

編集:この答えは彼の問題に完全には対処していません-私はここで他の提案をしました: C++文字列操作

彼の問題は、彼が作成したchar *のスコープを関数の外部に拡張し、非同期ジョブが終了するまで拡張する必要があることです。

元の回答:

C ++では、標準ライブラリまたはBoostを使用できない場合でも、次のようなクラスがあります。

template<class T>
class ArrayGuard {
  public:
    ArrayGuard(T* ptr) { _ptr = ptr; }
    ~ArrayGuard() { delete[] _ptr; }
  private:
    T* _ptr;
    ArrayGuard(const ArrayGuard&);
    ArrayGuard& operator=(const ArrayGuard&);
}

あなたはそれを次のように使用します:

char* buffer = new char[50];
ArrayGuard<char *> bufferGuard(buffer);

バッファはスコープの最後(リターンまたはスロー時)に削除されます。

スコープの最後で解放される静的サイズの配列のように扱いたい動的サイズの配列の単純な配列削除の場合。

シンプルに保ちましょう。より洗練されたスマートポインターが必要な場合は、Boostを使用してください。

これは、例の50が可変である場合に役立ちます。

于 2008-11-10T16:44:14.237 に答える
3

問題は、作成してLoadAndSetUpData()に渡した文字列をクリーンアップする方法です。

私はそれを仮定しています:

  1. LoadAndSetUpData()は独自のコピーを作成しません
  2. LoadAndSetUpData()を変更してそれを行うことはできません
  3. LoadAndSetupData()が戻った後も、しばらくの間文字列が存在する必要があります

ここに提案があります:

  1. 呼び出される独自のキューオブジェクトを作成できますか?それらはあなたの文字列を使用するものの後に呼び出されることが保証されていますか?その場合は、delete[]を呼び出すのと同じ文字列を使用してクリーンアップキューイベントを作成します

  2. 信頼できる最大数はありますか?大量の文字列を作成した場合、それらを1つのサイクルで使用して、最初に戻ったときにその文字列を再利用しても問題がないことを確認できますか?

  3. 頼りにできる時間はありますか?もしそうなら、どこかで削除のためにそれらを登録し、しばらくしてからそれを確認してください。

最良の方法は、char*を使用して所有権を取得またはコピーする関数です。共有所有権は、参照カウントやガベージコレクションなしで行うのが最も難しいことです。

于 2008-11-10T16:56:44.570 に答える
3

C++ のメモリ管理で覚えておくべきことは、所有権です。LoadAndSetupData データが文字列の所有権を取得しない場合でも、それはユーザーの責任です。(非同期性の問題のため) すぐに削除することはできないため、削除できることがわかるまで、これらのポインターを保持する必要があります。

作成した文字列のプールを維持します。

  • キューが完全に処理されたことがわかっている時点がある場合は、プール内のすべての文字列を単純に削除できます。
  • 特定の時点以降に作成されたすべての文字列が処理されていることがわかっている場合は、文字列がいつ作成されたかを追跡し、そのサブセットを削除できます。- 個々の文字列がいつ処理されたかを何らかの方法で確認できる場合は、その文字列を削除してください。

class StringPool
{
    struct StringReference {
        char *buffer;
        time_t created;
    } *Pool;

    size_t PoolSize;
    size_t Allocated;

    static const size_t INITIAL_SIZE = 100;

    void GrowBuffer()
    {
        StringReference *newPool = new StringReference[PoolSize * 2];
        for (size_t i = 0; i < Allocated; ++i)
            newPool[i] = Pool[i];
        StringReference *oldPool = Pool;
        Pool = newPool;
        delete[] oldPool;
    }

public:

    StringPool() : Pool(new StringReference[INITIAL_SIZE]), PoolSize(INITIAL_SIZE)
    {
    }

    ~StringPool()
    {
        ClearPool();
        delete[] Pool;
    }

    char *GetBuffer(size_t size)
    {
        if (Allocated == PoolSize)
            GrowBuffer();
        Pool[Allocated].buffer = new char[size];
        Pool[Allocated].buffer = time(NULL);
        ++Allocated;
    }

    void ClearPool()
    {
        for (size_t i = 0; i < Allocated; ++i)
            delete[] Pool[i].buffer;
        Allocated = 0;
    }

    void ClearBefore(time_t knownCleared)
    {
        size_t newAllocated = 0;
        for (size_t i = 0; i < Allocated; ++i)
        {
            if (Pool[i].created < knownCleared)
            {
                delete[] Pool[i].buffer;
            }
            else
            {
                Pool[newAllocated] = Pool[i];
                ++newAllocated;
            }
        }
        Allocated = newAllocated;
    }

    // This compares pointers, not strings!
    void ReleaseBuffer(char *knownCleared)
    {
        size_t newAllocated = 0;
        for (size_t i = 0; i < Allocated; ++i)
        {
            if (Pool[i].buffer == knownCleared)
            {
                delete[] Pool[i].buffer;
            }
            else
            {
                Pool[newAllocated] = Pool[i];
                ++newAllocated;
            }
        }
        Allocated = newAllocated;
    }

};
于 2008-11-10T16:57:27.297 に答える
2

char *を使用する必要がある場合、LoadAndSetupData()は、呼び出し後にchar*のメモリを所有しているユーザーを明示的に文書化する必要があります。次の2つのいずれかを実行できます。

  1. 文字列をコピーします。 これはおそらく最も簡単なことです。LoadAndSetupDataは文字列を内部バッファにコピーし、呼び出し元は常にメモリの責任を負います。

  2. 所有権を譲渡します。 LoadAndSetupData()は、char*のメモリを最終的に解放する責任があることを文書化します。呼び出し元は、メモリの解放について心配する必要はありません。

文字列のアロケータもそれを解放する責任があるので、私は一般的に#1のように安全なコピーを好みます。#2を使用する場合、アロケータは物事を解放しないことを覚えておく必要があります。メモリ管理は2つの場所で行われるため、保守が困難です。いずれの場合も、発信者が何を期待できるかがわかるように、ポリシーを明示的に文書化する必要があります。

#1を使用する場合は、Lou Francoの回答を見て、例外安全でchar []を割り当てる方法を確認してください。必ず、ガードクラスを使用して解放してください。配列にstd::auto_ptrを(安全に)使用できないことに注意してください。

于 2008-11-10T16:52:14.780 に答える
2

std::string はオプションではないため、何らかの理由でスマート ポインターを調べましたか? ブーストを見る

ただし、std::string の使用をお勧めすることしかできません。

キリスト教徒

于 2008-11-10T16:41:20.013 に答える
2

関数の後に nameToUse がまだ存在している必要があるため、new を使用してスタックしています。私が行うことは、それへのポインターを返すことです。そのため、呼び出し元は後で不要になったときにそれを「削除」できます。

char * LoadStuff()
{
    char* suffix = GetSuffix();
    char* nameToUse = new char[50];
    sprintf("%s%s",mBaseName,suffix);

    LoadAndSetupData(nameToUse);
    return nameToUse;
}

それから:

char *name = LoadStuff();
// do whatever you need to do:
delete [] name;
于 2008-11-10T16:41:53.663 に答える
1

この場合、ヒープに割り当てる必要はありません。そして常にsnprintfを使用してください:

char nameToUse[50];
snprintf(nameToUse, sizeof(nameToUse), "%s%s",mBaseName,suffix);
于 2008-11-10T16:58:42.397 に答える
0

LoadStuffの範囲を超えてnameToUseが正確に使用されているのはどこですか?LoadStuffの後に誰かがそれを必要とする場合は、メモリの割り当て解除の責任とともに、それを渡す必要があります

あなたが提案したようにc#でそれをしたとしたら

LoadAndSetupData(mBaseName + GetSuffix()); 

その場合、LoadAndSetupDataのパラメーターを参照するものはないため、安全に次のように変更できます。

char nameToUse[50];

マーティンが示唆したように。

于 2008-11-10T16:42:27.360 に答える
0

nameToUseに割り当てるメモリの存続期間を管理する必要があります。std :: stringなどのクラスにまとめると、生活が少し簡単になります。

これは小さな怒りだと思いますが、あなたの問題に対するこれ以上の解決策は考えられないので、別の潜在的な問題を指摘します。文字列をコピーまたは連結するときは、書き込むバッファのサイズを注意深く確認する必要があります。strcat、strcpy、sprintfなどの関数は、ターゲットバッファーの終わりを簡単に上書きして、誤ったランタイムエラーやセキュリティの脆弱性を引き起こす可能性があります。

申し訳ありませんが、私自身の経験は主にWindowsプラットフォームであり、strcat_s、strcpy_s、およびsprintf_sと呼ばれるこれらの関数の「安全な」バージョンが導入されました。同じことが、関連する多くの機能にも当てはまります。

于 2008-11-10T16:46:56.197 に答える
0

最初に:割り当てられた文字列がLoadStuff()の終わりを超えて持続する必要があるのはなぜですか?その要件を削除するためにリファクタリングできる方法はありますか?

C ++はこの種のことを行う簡単な方法を提供しないため、ほとんどのプログラミング環境では、ポインターに関する一連のガイドラインを使用して、削除/解放の問題を防ぎます。割り当て/解放できるのは1回だけなので、ポインタを「所有」しているのは誰かを明確にする必要があります。いくつかのサンプルガイドライン:

1)通常、文字列を割り当てる人が所有者であり、文字列を解放する責任もあります。

2)割り当てたものとは異なる関数/クラスで解放する必要がある場合は、別のクラス/関数への所有権の明示的な受け渡しが必要です。

3)特に明記されていない限り、ポインタ(文字列を含む)は呼び出し元に属します。関数、コンストラクターなどは、取得した文字列ポインターが関数呼び出しの終了後も存続すると想定することはできません。ポインタの永続的なコピーが必要な場合は、strdup()を使用してローカルコピーを作成する必要があります。

これが特定のケースで要約すると、LoadStuff()はnameToUseを削除する必要があり、それが呼び出す関数はローカルコピーを作成する必要があります。

1つの代替ソリューション:nameToUseが多くの場所に渡され、プログラムの存続期間中存続する必要がある場合は、それをグローバル変数にすることができます。(これにより、コピーを大量に作成する手間が省けます。)グローバル名前空間を汚染したくない場合は、関数に対して静的ローカルとして宣言するだけで済みます。

static char *nameToUse = new char[50];
于 2008-11-10T16:49:36.503 に答える
0

皆様、ご回答ありがとうございます。この問題には具体的な解決策がなく、それに関する最良の議論はすべて私や他の人が賛成しているため、「答え」として1つを選択していません.

あなたの提案はすべて良いものであり、私の質問のぎこちなさに非常に辛抱強く対応してくれました。ご覧のとおり、これはより複雑な問題を単純化したものであり、私が示した例に関連してさらに多くのことが進行しているため、その一部が完全に意味を成していない可能性があります。

あなたの興味のために、私は今のところ困難から抜け出す方法を「ごまかす」ことに決めました。ベース名は恣意的だと​​言いましたが、そうではありません。実際、それらは限定された名前のセットでもあり、ある時点で変更される可能性のある限定されたセットであるため、より一般的な問題を解決しようとしていました.

とりあえず、「静的」ソリューションをサフィックスに拡張し、可能な名前のテーブルを作成します。これは非常に「ハッキー」ですが、機能し、さらに、私ができない大量の複雑なコードのリファクタリングを回避できます。

フィードバックは素晴らしいものでした。どうもありがとうございました。

于 2008-11-10T17:35:26.287 に答える
0

ここでいくつかのアイデアを組み合わせることができます。

アプリケーションをどのようにモジュール化したかによって、nameToUse が固定サイズのローカル変数として定義できるスコープを実行が決定するメソッド (main?) が存在する場合があります。ローカル変数を持つ関数が終了するか、またはプログラムは他の方法で終了します。

これと、動的な割り当てと削除を使用する場合との間にほとんど違いはありません (位置を保持するポインターは、多かれ少なかれ同じ方法で管理する必要があるため)。多くの場合、ローカル割り当てはより直接的であり、必要な最大有効期間を特定の関数の実行期間に関連付けることに問題がない場合、非常に安価です。

于 2008-11-10T19:09:41.837 に答える
-1

LoadAndSetupData がどこで定義されているかは完全にはわかりませんが、文字列の独自のコピーを保持しているようです。そのため、LoadAndSetupData の呼び出し後にローカルに割り当てられたコピーを削除し、独自のコピーを管理できるようにする必要があります。

または、LoadAndSetupData が、割り当てられた char[] をクリーンアップすることを確認してください。

私の好みは、他の関数に独自のコピーを保持させ、それを管理させて、別のクラスにオブジェクトを割り当てないようにすることです。

編集: 固定サイズ [50] で new を使用するため、提案されているようにローカルにして、LoadAndSetupData に独自のコピーを作成させることもできます。

于 2008-11-10T16:41:35.270 に答える