1

メモリ セクションを解放しようとすると、プログラムがクラッシュするようです。以下は、リンクされたリストの全体的な構造です。

typedef struct {
    char                            *dataitem;
    struct listelement              *link;
    int16_t                         wordSize;
    int16_t                         (*libWord)[Q];
    char                            gpioValue;
    struct listelement              *syllables;
}listelement;

この関数を呼び出すと、プログラムがクラッシュします。

recordedWordsPointer = RemoveItem(recordedWordsPointer);                            // get rid of any junk stored in the recorded buffer

どこ：

volatile listelement *recordedWordsPointer;

libWord に値を格納しており、別のリンクがある場合は次のリンクを指し、そうでない場合は NULL を指します。以下は、関数に入ったときに何が起こるかを示しています。

listelement * RemoveItem (listelement * listpointer) {
    cpu_irq_disable();
    listelement * tempp = listpointer;

    while( listpointer->syllables != NULL ){
        RemoveSyllable(listpointer->syllables);
    }
    if( listpointer != NULL ){
        tempp = listpointer -> link;
        free (listpointer->dataitem);
        free (listpointer->libWord);
        free (listpointer);
    }
    cpu_irq_enable();   
    return tempp;
}

void RemoveSyllable (listelement * listpointer) {

    while( listpointer->syllables != NULL ){
        RemoveSyllable(listpointer->syllables);
    }
    free (listpointer->dataitem);
    free (listpointer->libWord);
    free (listpointer);
    listpointer = NULL;
    return;
}

メモリクラッシュを引き起こすために何か間違ったことをしているのだろうかと思っていましたか?

ありがとう！

編集：

私は、メモリの場所を構築する方法を示すように求められました. 次の 2 つの関数を使用します。

listelement * AddItem (listelement * listpointer, char* name, int16_t size, int16_t wordLength, int16_t (*words)[Q]) {
    // returns listPointer at the beginning of list
    listelement * lp = listpointer;
    listelement * listPointerTemp;
    char ErrorHandler = NULL;
    // are we at the end of the list?
    if (listpointer != NULL) {
        // move down to the end of the list
        while (listpointer -> link != NULL)
        listpointer = listpointer -> link;
        listPointerTemp = listpointer;
        listpointer -> link = (struct listelement  *) malloc (sizeof (listelement));
        // on fail end links becomes NULL already above
        if(listpointer -> link != NULL){
            listpointer = listpointer -> link;
            listpointer -> link = NULL;
            listpointer -> wordSize = wordLength;
            listpointer -> syllables = NULL;

            listpointer -> dataitem = (char*) malloc ((size + 1)*sizeof(char));
            if(listpointer -> dataitem != NULL){
                for(int i=0; i<size ; i++){
                    listpointer -> dataitem[i] = name[i];
                }
                listpointer -> dataitem[size] = NULL;

                listpointer -> libWord =  (int16_t(*)[Q])malloc(wordLength*Q*sizeof(int16_t));
                if(listpointer -> libWord != NULL){
                    for (int16_t row=0 ; row < wordLength ; row++){
                        for (int col=0 ; col < Q ; col++){
                            listpointer -> libWord[row][col]  = words[row][col];
                        }
                    }
                    ErrorHandler = 1;
                }else{
                    free(listpointer->dataitem);
                    free(listpointer);
                    listPointerTemp -> link = NULL;
                }
            }else{
                free(listpointer);
                listPointerTemp -> link = NULL;
            }
        }
        if(ErrorHandler == NULL){
            //failure
            usart_write_line(&AVR32_USART0,"\r\n--------------------------------------------\r\n");
            usart_write_line(&AVR32_USART0,"Ran out of Memory!  Word not created.\r\n");
            usart_write_line(&AVR32_USART0,"\r\n--------------------------------------------\r\n");
        }
        return lp;
    } else {
        listpointer = (struct listelement  *) malloc (sizeof (listelement));

        if(listpointer != NULL){
            listpointer -> link = NULL;
            listpointer -> wordSize = wordLength;
            listpointer -> syllables = NULL;

            listpointer -> dataitem = (char*) malloc ((size + 1)*sizeof(char));
            if(listpointer -> dataitem != NULL){
                for(int16_t i=0; i<size ; i++){
                    listpointer -> dataitem[i] = name[i];
                }
                listpointer -> dataitem[size] = NULL;

                listpointer -> libWord =  (int16_t(*)[Q])malloc(wordLength*Q*sizeof(int16_t));
                if(listpointer -> libWord != NULL){
                    for (int16_t row=0 ; row < wordLength ; row++){
                        for (int col=0 ; col < Q ; col++){
                            listpointer -> libWord[row][col]  = words[row][col];
                        }
                    }
                    ErrorHandler = 1;
                }else{
                    free(listpointer->dataitem);
                    free(listpointer);
                    listPointerTemp -> link = NULL;
                }
            }else{
                free(listpointer);
                listPointerTemp -> link = NULL;
            }
        }
        if(ErrorHandler == NULL){
            //failure
            usart_write_line(&AVR32_USART0,"\r\n--------------------------------------------\r\n");
            usart_write_line(&AVR32_USART0,"Ran out of Memory!  Word not created.\r\n");
            usart_write_line(&AVR32_USART0,"\r\n--------------------------------------------\r\n");
        }
        return listpointer;
    }
}

listelement* AddSyllable (listelement * listpointer, char* name, int16_t size, int16_t wordLength, int16_t (*words)[Q]) {
    // returns listPointer at the beginning of list
    listelement * lp = listpointer;
    listelement * listPointerTemp;
    char ErrorHandler = NULL;
    // are we at the end of the list?
    if (listpointer != NULL) {
        // move down to the end of the list
        while (listpointer -> syllables != NULL)
        listpointer = listpointer -> syllables;
        listPointerTemp = listpointer;
        listpointer -> syllables = (struct listelement  *) malloc (sizeof (listelement));
        // on fail end links becomes NULL already above
        if(listpointer -> syllables != NULL){
            listpointer = listpointer -> syllables;
            listpointer -> link = NULL;
            listpointer -> wordSize = wordLength;

            listpointer -> dataitem = (char*) malloc ((size + 1)*sizeof(char));
            if(listpointer -> dataitem != NULL){
                for(int i=0; i<size ; i++){
                    listpointer -> dataitem[i] = name[i];
                }
                listpointer -> dataitem[size] = NULL;

                listpointer -> libWord =  (int16_t(*)[Q])malloc(wordLength*Q*sizeof(int16_t));
                if(listpointer -> libWord != NULL){
                    for (int16_t row=0 ; row < wordLength ; row++){
                        for (int col=0 ; col < Q ; col++){
                            listpointer -> libWord[row][col]  = words[row][col];
                        }
                    }
                    ErrorHandler = 1;
                }else{
                    free(listpointer->dataitem);
                    free(listpointer);
                    listPointerTemp -> syllables = NULL;
                }
            }else{
                free(listpointer);
                listPointerTemp -> syllables = NULL;
            }
        }
        if(ErrorHandler == NULL){
            //failure
            usart_write_line(&AVR32_USART0,"\r\n--------------------------------------------\r\n");
            usart_write_line(&AVR32_USART0,"Ran out of Memory!  Word not created.\r\n");
            usart_write_line(&AVR32_USART0,"\r\n--------------------------------------------\r\n");
        }
        return lp;
    } else {
        listpointer = (struct listelement  *) malloc (sizeof (listelement));

        if(listpointer != NULL){
            listpointer -> link = NULL;
            listpointer -> wordSize = wordLength;

            listpointer -> dataitem = (char*) malloc ((size + 1)*sizeof(char));
            if(listpointer -> dataitem != NULL){
                for(int16_t i=0; i<size ; i++){
                    listpointer -> dataitem[i] = name[i];
                }
                listpointer -> dataitem[size] = NULL;

                listpointer -> libWord =  (int16_t(*)[Q])malloc(wordLength*Q*sizeof(int16_t));
                if(listpointer -> libWord != NULL){
                    for (int16_t row=0 ; row < wordLength ; row++){
                        for (int col=0 ; col < Q ; col++){
                            listpointer -> libWord[row][col]  = words[row][col];
                        }
                    }
                    ErrorHandler = 1;
                }else{
                    free(listpointer->dataitem);
                    free(listpointer);
                    listPointerTemp -> syllables = NULL;
                }
            }else{
                free(listpointer);
                listPointerTemp -> syllables = NULL;
            }
        }
        if(ErrorHandler == NULL){
            //failure
            usart_write_line(&AVR32_USART0,"\r\n--------------------------------------------\r\n");
            usart_write_line(&AVR32_USART0,"Ran out of Memory!  Word not created.\r\n");
            usart_write_line(&AVR32_USART0,"\r\n--------------------------------------------\r\n");
        }
        return listpointer;
    }
}
4

2 に答える 2

2

syllablesあなたの RemoveSyllable 関数は、実際にはメンバーを NULLに設定しません。ルーチン内ではそうすると思いますが、実際にはローカル変数内で値を変更しているだけです。

于 2013-07-21T19:41:56.213 に答える
1

初期観察

これは、完全な回答というよりも、コードの部分的な分解です。

コンパイラの警告をオンにしていませんか? そうでない場合、なぜですか？

typedef struct {
    char                            *dataitem;
    struct listelement              *link;
    int16_t                         wordSize;
    int16_t                         (*libWord)[Q];
    char                            gpioValue;
    struct listelement              *syllables;
} listelement

linkとのメンバーは、あなたが である匿名を指しsyllablesていないことに注意してください。それらは、他のまったく無関係な構造を指しています。コンパイラはこれについて激しく不満を言っています — もっともな理由があります. 修正は簡単です: に変更します。しかし、コード内でそのような混乱を処理する必要はありません。structtypedeflistelementtypedef struct {typedef struct listelement {

を誤用することもありNULLます。たとえばchar ErrorHandler = NULL;、ポインタが異なるサイズの整数に変換されているという警告を生成します。 NULL必ずしも 0 ではありません。それは少なくとも時々((void *)0)または似たようなものです。また、代わりに NULL を使用し'\0'て、文字列の最後のバイトを null バイトに設定します — これもコンパイラの警告を引き出します。これらはひどく深刻というよりはうるさいですが、最小限のキャストで静かにコンパイルされるコードを目指す必要があります (つまり、コンパイラの警告を黙らせるためにどこでもキャストを平手打ちしないでください)。

あなたが書く：

    if(listpointer -> link != NULL){

ブレースの配置を無視して (これは正当な議論の対象となります)、 の後にスペースを入れ、 のif周りにスペースを入れないでください->。スペースに関する同様のコメントがwhile;に適用されます。呼び出しと定義の両方で、関数名と開き括弧の間にスペースを使用しないことも慣習的です。

この警告は重大です:

usart.c:297:5: warning: passing argument 1 of ‘RemoveItem’ discards ‘volatile’ qualifier from pointer target type [enabled by default]
usart.c:253:14: note: expected ‘struct listelement *’ but argument is of type ‘volatile struct listelement *’

recordedWordsPointer基本的に、を呼び出すときのボラティリティを取り除きますRemoveItem。これにより、 の存在がvolatile不要になります。である場合はvolatile、それを使用するすべての場所でそれが認識されていることを確認する必要があります。または、より単純に、volatile修飾子を削除します。

AddItem()とのそれぞれに大量のコード重複があるようですAddSyllable()。各機能の約半分を排除することを目指す必要があります。

ここで、何が問題なのかを確認できるように、関数を実際にどのように使用するかを検討する必要があります。AddItem()特にとAddSyllable()関数への引数に神秘的な が含まれているため、これは困難int16_t (*words)[Q]です。main()これらの関数を適切な引数 (ファイルから読み取った値ではなく、おそらく初期化されたデータ構造である必要があります) で呼び出す簡単なプログラムを示していただければ助かります。

リファクタリングされたコード

コード量を削減するための重要なリファクタリングの 1 つは、listelement指定された名前、サイズ、単語数、および単語を作成する関数を作成することです。以下のdupstr()関数は と同一ではありませんstrdup()。長さを取り、おそらくより長い文字列の多くの文字を複製します。

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

enum { Q = 16 };

typedef struct listelement
{
    char                *dataitem;
    struct listelement  *link;
    int16_t              wordSize;
    int16_t            (*libWord)[Q];
    struct listelement  *syllables;
} listelement;

static int AVR32_USART0 = 0;

static void cpu_irq_disable(void);
static void cpu_irq_enable(void);   
static void usart_write_line(int *ptr, char *msg);
extern void RemoveSyllable(listelement *listpointer);
extern listelement *RemoveItem(listelement *listpointer);
extern listelement *AddSyllable(listelement *listpointer, char *name, int16_t size, int16_t wordLength, int16_t (*words)[Q]);
extern listelement *AddItem(listelement *listpointer, char *name, int16_t size, int16_t wordLength, int16_t (*words)[Q]);

/* Duplicate string - or use POSIX strdup() */
static char *dupstr(const char *name, int16_t size)
{
    char *str = malloc(size+1);
    if (str != NULL)
    {
        memmove(str, name, size);
        str[size] = '\0';
    }
    return str;
}

static listelement *makeElement(const char *name, int16_t size, int16_t wordLength, int16_t (*words)[Q])
{
    listelement *listpointer = (listelement *)malloc(sizeof(listelement));
    // on fail end links becomes NULL already above
    if (listpointer != NULL)
    {
        listpointer->dataitem = dupstr(name, size);
        listpointer->libWord =  (int16_t(*)[Q])malloc(wordLength*Q*sizeof(int16_t));
        if (listpointer->dataitem == NULL || listpointer->libWord == 0)
        {
            free(listpointer->dataitem);
            free(listpointer->libWord);
            free(listpointer);
            listpointer = NULL;
        }
        else
        {
            listpointer->link = NULL;
            listpointer->wordSize = wordLength;
            listpointer->syllables = NULL;
            for (int16_t row=0; row < wordLength; row++)
            {
                for (int col=0; col < Q; col++)
                {
                    listpointer->libWord[row][col] = words[row][col];
                }
            }
        }
    }
    return listpointer;
}

static void reportError(void)
{
    usart_write_line(&AVR32_USART0,"\r\n--------------------------------------------\r\n");
    usart_write_line(&AVR32_USART0,"Ran out of Memory!  Word not created.\r\n");
    usart_write_line(&AVR32_USART0,"\r\n--------------------------------------------\r\n");
}

listelement *AddItem(listelement *listpointer, char *name, int16_t size, int16_t wordLength, int16_t (*words)[Q])
{
    listelement *lp = listpointer;
    char ErrorHandler = 0;
    if (listpointer != NULL)
    {
        while (listpointer->link != NULL)
            listpointer = listpointer->link;
        listpointer->link = makeElement(name, size, wordLength, words);
        if (listpointer->link != NULL)
        {
            listpointer = listpointer->link;
            ErrorHandler = 1;
        }
    }
    else
    {
        listpointer = makeElement(name, size, wordLength, words);
        if (listpointer != NULL)
            ErrorHandler = 1;
        lp = listpointer;
    }
    if (ErrorHandler == 0)
        reportError();
    return lp;
}

listelement *AddSyllable(listelement *listpointer, char *name, int16_t size, int16_t wordLength, int16_t (*words)[Q])
{
    listelement *lp = listpointer;
    char ErrorHandler = 0;
    if (listpointer != NULL)
    {
        while (listpointer->syllables != NULL)
            listpointer = listpointer->syllables;
        listpointer->syllables = makeElement(name, size, wordLength, words);
        if (listpointer->syllables != NULL)
            ErrorHandler = 1;
    }
    else 
    {
        listpointer = makeElement(name, size, wordLength, words);
        if (listpointer != NULL)
            ErrorHandler = 1;
        lp = listpointer;
    }
    if (ErrorHandler == 0)
        reportError();
    return lp;
}

listelement *RemoveItem(listelement *listpointer)
{
    cpu_irq_disable();
    listelement * tempp = listpointer;

    while (listpointer->syllables != NULL)
    {
        RemoveSyllable(listpointer->syllables);
    }
    if (listpointer != NULL)
    {
        tempp = listpointer->link;
        free (listpointer->dataitem);
        free (listpointer->libWord);
        free (listpointer);
    }
    cpu_irq_enable();   
    return tempp;
}

void RemoveSyllable(listelement *listpointer)
{
    while (listpointer->syllables != NULL)
    {
        RemoveSyllable(listpointer->syllables);
    }
    free(listpointer->dataitem);
    free(listpointer->libWord);
    free(listpointer);
    listpointer = NULL;
    return;
}

static void cpu_irq_disable(void) { AVR32_USART0 = 0; }
static void cpu_irq_enable(void)  { AVR32_USART0 = 1; }   
static void usart_write_line(int *ptr, char *msg)
{
    *ptr = !*ptr;
    fprintf(stderr, "%s", msg);
}

int main(void)
{
    listelement *recordedWordsPointer = 0;
    recordedWordsPointer = RemoveItem(recordedWordsPointer);
}

addSyllable()そこにあるすべての変更が適切であることを証明したわけではありませんが、 and関数は、繰り返される作業のほとんどを関数addItem()が行うことで、ヒープ全体が読みやすくなることは確信しています。makeElement()間違いなく改善の余地があります。コードには、機能を実際に実行する作業が必要main()です (そして、上記のコードは適格ではありません)。

配列へのポインターがどのように処理されるかについては留保がありますが、実際に実行できるマシンにコードを持ち込んでいvalgrindません (残念ながら、Mac OS X 10.8.x はまだサポートされvalgrindていません)。私はそれが間違っていることを証明していません。私はそれが間違っているのではないかと疑っています。呼び出しコードと、配列へのポインターとして渡された変数の定義を確認することは、私を大いに助けてくれます。

于 2013-07-22T02:09:17.280 に答える