Try   HackMD

Exercise 17

原始

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

#define MAX_DATA 512
#define MAX_ROWS 100

struct Address {
    int id;
    int set;
    char name[MAX_DATA];
    char email[MAX_DATA];
};

struct Database {
    struct Address rows[MAX_ROWS];
};

struct Connection {
    FILE *file;
    struct Database *db;
};

void die(const char *message)
{
    if (errno) {
        perror(message);
    } else {
        printf("ERROR: %s\n", message);
    }

    exit(1);
}

void Address_print(struct Address *addr)
{
    printf("%d %s %s\n", addr->id, addr->name, addr->email);
}

void Database_load(struct Connection *conn)
{
    int rc = fread(conn->db, sizeof(struct Database), 1, conn->file);
    if (rc != 1)
        die("Failed to load database.");
}

struct Connection *Database_open(const char *filename, char mode)
{
    struct Connection *conn = malloc(sizeof(struct Connection));
    if (!conn)
        die("Memory error");

    conn->db = malloc(sizeof(struct Database));
    if (!conn->db)
        die("Memory error");

    if (mode == 'c') {
        conn->file = fopen(filename, "w");
    } else {
        conn->file = fopen(filename, "r+");

        if (conn->file) {
            Database_load(conn);
        }
    }

    if (!conn->file)
        die("Failed to open the file");

    return conn;
}

void Database_close(struct Connection *conn)
{
    if (conn) {
        if (conn->file)
            fclose(conn->file);
        if (conn->db)
            free(conn->db);
        free(conn);
    }
}

void Database_write(struct Connection *conn)
{
    rewind(conn->file);

    int rc = fwrite(conn->db, sizeof(struct Database), 1, conn->file);
    if (rc != 1)
        die("Failed to write database.");

    rc = fflush(conn->file);
    if (rc == -1)
        die("Cannot flush database.");
}

void Database_create(struct Connection *conn)
{
    int i = 0;

    for (i = 0; i < MAX_ROWS; i++) {
        // make a prototype to initialize it
        struct Address addr = {.id = i, .set = 0};
        // then just assign it
        conn->db->rows[i] = addr;
    }
}

void Database_set(struct Connection *conn, int id, const char *name,
        const char *email)
{
    struct Address *addr = &conn->db->rows[id];
    if (addr->set)
        die("Already set, delete it first");

    addr->set = 1;
    // WARNING: bug, read the "How To Break It" and fix this
    char *res = strncpy(addr->name, name, MAX_DATA);
    // demonstrate the strncpy bug
    if (!res)
        die("Name copy failed");

    res = strncpy(addr->email, email, MAX_DATA);
    if (!res)
        die("Email copy failed");
}

void Database_get(struct Connection *conn, int id)
{
    struct Address *addr = &conn->db->rows[id];

    if (addr->set) {
        Address_print(addr);
    } else {
        die("ID is not set");
    }
}

void Database_delete(struct Connection *conn, int id)
{
    struct Address addr = {.id = id, .set = 0};
    conn->db->rows[id] = addr;
}

void Database_list(struct Connection *conn)
{
    int i = 0;
    struct Database *db = conn->db;

    for (i = 0; i < MAX_ROWS; i++) {
        struct Address *cur = &db->rows[i];

        if (cur->set) {
            Address_print(cur);
        }
    }
}

int main(int argc, char *argv[])
{
    if (argc < 3)
        die("USAGE: ex17 <dbfile> <action> [action params]");

    char *filename = argv[1];
    char action = argv[2][0];
    struct Connection *conn = Database_open(filename, action);
    int id = 0;

    if (argc > 3) id = atoi(argv[3]);
    if (id >= MAX_ROWS) die("There's not that many records.");

    switch (action) {
        case 'c':
            Database_create(conn);
            Database_write(conn);
            break;

        case 'g':
            if (argc != 4)
                die("Need an id to get");

            Database_get(conn, id);
            break;

        case 's':
            if (argc != 6)
                die("Need id, name, email to set");

            Database_set(conn, id, argv[4], argv[5]);
            Database_write(conn);
            break;

        case 'd':
            if (argc != 4)
                die("Need id to delete");

            Database_delete(conn, id);
            Database_write(conn);
            break;

        case 'l':
            Database_list(conn);
            break;
        default:
            die("Invalid action: c=create, g=get, s=set, d=del, l=list");
    }

    Database_close(conn);

    return 0;
}

這個程式的功能在建立一個基本聯絡資訊的資料庫 struct Database, 這個資料庫包含一個 elementstruct Address rows[MAX_ROWS]
struct Address 包含 int idint setchar name[MAX_DATA]char email[MAX_DATA] , 其中 MAX_ROWS = 1000, MAX_DATA = 512 。
程式使用方式:

./ex17 <dbfile> <action> [action params]

 <dbfile> : 資料庫檔名
 <action> : 執行動作
     c : 建立資料庫
     g <id> : 取得某筆 id 的資料並印出
     s <id> <name> <email> : 設定第 <id> 筆資料的內容
     d <id> : 刪除第 <id> 筆資料,這裡實作把 <set> 改為 0
     l : 列出所有建立的 Address
 [action params] : 其他內容
     <action> 之後接的參數

Extra Credit

  • The die function needs to be augmented to let you pass the conn
    variable, so it can close it and clean up.
    -> 完成

ex17_ext1

#include <stdio.h> #include <assert.h> #include <stdlib.h> #include <errno.h> #include <string.h> #define MAX_DATA 512 #define MAX_ROWS 100 struct Address { int id; int set; char name[MAX_DATA]; char email[MAX_DATA]; }; struct Database { struct Address rows[MAX_ROWS]; }; struct Connection { FILE *file; struct Database *db; }; void Database_close(struct Connection *conn); void die(const char *message, struct Connection *conn) { if (errno) { if (conn) { Database_close(conn); } perror(message); } else { printf("ERROR: %s\n", message); if (conn) { Database_close(conn); } } exit(1); } void Address_print(struct Address *addr) { printf("%d %s %s\n", addr->id, addr->name, addr->email); } void Database_load(struct Connection *conn) { int rc = fread(conn->db, sizeof(struct Database), 1, conn->file); if (rc != 1) die("Failed to load database.", conn); } struct Connection *Database_open(const char *filename, char mode) { struct Connection *conn = malloc(sizeof(struct Connection)); if (!conn) die("Memory error", conn); conn->db = malloc(sizeof(struct Database)); if (!conn->db) die("Memory error", conn); if (mode == 'c') { conn->file = fopen(filename, "w"); } else { conn->file = fopen(filename, "r+"); if (conn->file) { Database_load(conn); } } if (!conn->file) die("Failed to open the file", conn); return conn; } void Database_close(struct Connection *conn) { if (conn) { if (conn->file) fclose(conn->file); if (conn->db) free(conn->db); free(conn); } } void Database_write(struct Connection *conn) { rewind(conn->file); int rc = fwrite(conn->db, sizeof(struct Database), 1, conn->file); if (rc != 1) die("Failed to write database.", conn); rc = fflush(conn->file); if (rc == -1) die("Cannot flush database.", conn); } void Database_create(struct Connection *conn) { int i = 0; for (i = 0; i < MAX_ROWS; i++) { // make a prototype to initialize it struct Address addr = {.id = i, .set = 0}; // then just assign it conn->db->rows[i] = addr; } } void Database_set(struct Connection *conn, int id, const char *name, const char *email) { struct Address *addr = &conn->db->rows[id]; if (addr->set) die("Already set, delete it first", conn); addr->set = 1; // WARNING: bug, read the "How To Break It" and fix this char *res = strncpy(addr->name, name, MAX_DATA); // bug fixed addr->name[MAX_DATA - 1] = '\0'; // demonstrate the strncpy bug if (!res) die("Name copy failed", conn); res = strncpy(addr->email, email, MAX_DATA); if (!res) die("Email copy failed", conn); // bug fixed addr->email[MAX_DATA - 1] = '\0'; } void Database_get(struct Connection *conn, int id) { struct Address *addr = &conn->db->rows[id]; if (addr->set) { Address_print(addr); } else { die("ID is not set", conn); } } void Database_delete(struct Connection *conn, int id) { struct Address addr = {.id = id, .set = 0}; conn->db->rows[id] = addr; } void Database_list(struct Connection *conn) { int i = 0; struct Database *db = conn->db; for (i = 0; i < MAX_ROWS; i++) { struct Address *cur = &db->rows[i]; if (cur->set) { Address_print(cur); } } } int main(int argc, char *argv[]) { if (argc < 3) die("USAGE: ex17 <dbfile> <action> [action params]", NULL); char *filename = argv[1]; char action = argv[2][0]; struct Connection *conn = Database_open(filename, action); int id = 0; if (argc > 3) id = atoi(argv[3]); if (id >= MAX_ROWS) die("There's not that many record.", conn); switch(action) { case 'c': Database_create(conn); Database_write(conn); break; case 'g': if (argc != 4) die("Need an id to get", conn); Database_get(conn, id); break; case 's': if (argc != 6) die("Need id, name, email to set", conn); Database_set(conn, id, argv[4], argv[5]); Database_write(conn); break; case 'd': if (argc != 4) die("Need id to delete", conn); Database_delete(conn, id); Database_write(conn); break; case 'l': Database_list(conn); break; default: die("Invalid action: c=create, g=get, s=set, d=del, l=list", conn); } Database_close(conn); return 0; }
  • Change the code to accept parameters for MAX_DATA and MAX_ROWS,
    store them in the Database struct, and write that to the file, thus creating a database that can be arbitrarily sized.
    > 嘗試調整 MAX_ROWS 使其變成可變的任意大小,但是在執行以下指令時出現 Segmentation Fault

ex17_ext2

#include <stdio.h> #include <assert.h> #include <stdlib.h> #include <errno.h> #include <string.h> #define MAX_DATA 512 #define MAX_ROWS 100 struct Address { int id; int set; char name[MAX_DATA]; char email[MAX_DATA]; }; struct Database { int max_rows; struct Address rows[]; }; struct Connection { FILE *file; struct Database *db; }; void Database_close(struct Connection *conn); void die(const char *message, struct Connection *conn) { if (errno) { if (conn) { Database_close(conn); } perror(message); } else { printf("ERROR: %s\n", message); if (conn) { Database_close(conn); } } exit(1); } void Address_print(struct Address *addr) { printf("%d %s %s\n", addr->id, addr->name, addr->email); } void Database_load(struct Connection *conn) { int rc = fread(conn->db, sizeof(struct Database), 1, conn->file); if (rc != 1) die("Failed to load database.", conn); } struct Connection *Database_open(const char *filename, char mode, int max_rows) { struct Connection *conn = malloc(sizeof(struct Connection)); if (!conn) die("Memory error", conn); conn->db = malloc(sizeof(struct Database) + sizeof(char[max_rows])); if (!conn->db) die("Memory error", conn); if (mode == 'c') { conn->file = fopen(filename, "w"); } else { conn->file = fopen(filename, "r+"); if (conn->file) { Database_load(conn); } } if (!conn->file) die("Failed to open the file", conn); return conn; } void Database_close(struct Connection *conn) { if (conn) { if (conn->file) fclose(conn->file); if (conn->db) free(conn->db); free(conn); } } void Database_write(struct Connection *conn) { rewind(conn->file); int rc = fwrite(conn->db, sizeof(struct Database), 1, conn->file); if (rc != 1) die("Failed to write database.", conn); rc = fflush(conn->file); if (rc == -1) die("Cannot flush database.", conn); } void Database_create(struct Connection *conn, int max_rows) { int i = 0; for (i = 0; i < max_rows; i++) { // make a prototype to initialize it struct Address addr = {.id = i, .set = 0}; // then just assign it conn->db->rows[i] = addr; } } void Database_set(struct Connection *conn, int id, const char *name, const char *email) { struct Address *addr = &conn->db->rows[id]; if (addr->set) die("Already set, delete it first", conn); addr->set = 1; // WARNING: bug, read the "How To Break It" and fix this char *res = strncpy(addr->name, name, MAX_DATA); // bug fixed addr->name[MAX_DATA - 1] = '\0'; // demonstrate the strncpy bug if (!res) die("Name copy failed", conn); res = strncpy(addr->email, email, MAX_DATA); if (!res) die("Email copy failed", conn); // bug fixed addr->email[MAX_DATA - 1] = '\0'; } void Database_get(struct Connection *conn, int id) { struct Address *addr = &conn->db->rows[id]; if (addr->set) { Address_print(addr); } else { die("ID is not set", conn); } } void Database_delete(struct Connection *conn, int id) { struct Address addr = {.id = id, .set = 0}; conn->db->rows[id] = addr; } void Database_list(struct Connection *conn) { int i = 0; struct Database *db = conn->db; for (i = 0; i < MAX_ROWS; i++) { struct Address *cur = &db->rows[i]; if (cur->set) { Address_print(cur); } } } int main(int argc, char *argv[]) { if (argc < 3) die("USAGE: ex17 <dbfile> <action> [action params]", NULL); char *filename = argv[1]; char action = argv[2][0]; // adding this to setting max_rows int max_rows = 10; struct Connection *conn = Database_open(filename, action, max_rows); int id = 0; if (argc > 3) id = atoi(argv[3]); if (id >= MAX_ROWS) die("There's not that many record.", conn); switch(action) { case 'c': Database_create(conn, max_rows); Database_write(conn); break; case 'g': if (argc != 4) die("Need an id to get", conn); Database_get(conn, id); break; case 's': if (argc != 6) die("Need id, name, email to set", conn); Database_set(conn, id, argv[4], argv[5]); Database_write(conn); break; case 'd': if (argc != 4) die("Need id to delete", conn); Database_delete(conn, id); Database_write(conn); break; case 'l': Database_list(conn); break; default: die("Invalid action: c=create, g=get, s=set, d=del, l=list", conn); } Database_close(conn); return 0; }
$ ./ex17_ext2 db.dat c
Segmentation fault

與ext1比較,修改了:

$ diff ex17_ext1.c ex17_ext2.c
18c18,19
<     struct Address rows[MAX_ROWS];
---
>     int max_rows;
>     struct Address rows[];
57c58
< struct Connection *Database_open(const char *filename, char mode)
---
> struct Connection *Database_open(const char *filename, char mode, int max_rows)
63c64
<     conn->db = malloc(sizeof(struct Database));
---
>     conn->db = malloc(sizeof(struct Database) + sizeof(char[max_rows]));
107c108
< void Database_create(struct Connection *conn)
---
> void Database_create(struct Connection *conn, int max_rows)
111c112
<     for (i = 0; i < MAX_ROWS; i++) {
---
>     for (i = 0; i < max_rows; i++) {
113a115
> 
180c182,185
<     struct Connection *conn = Database_open(filename, action);
---
> 
>     // adding this to setting max_rows
>     int max_rows = 10;
>     struct Connection *conn = Database_open(filename, action, max_rows);
188c193
<             Database_create(conn);
---
>             Database_create(conn, max_rows);

使用gdb 把過程顯示出來

$ gdb --batch --ex run --ex bt --ex q --args ./ex17_ext2 db.dat c

Program received signal SIGSEGV, Segmentation fault.
0x0000007ff7eb1d88 in __GI_rewind (fp=0x55555632e0) at rewind.c:34
34	rewind.c: No such file or directory.
#0  0x0000007ff7eb1d88 in __GI_rewind (fp=0x55555632e0) at rewind.c:34
#1  0x0000005555550e3c in Database_write (conn=0x55555632a0) at ex17_ext2.c:97
#2  0x00000055555512d0 in main (argc=3, argv=0x7ffffff998) at ex17_ext2.c:194
A debugging session is active.

	Inferior 1 [process 1305537] will be killed.

Quit anyway? (y or n) [answered Y; input not from terminal]

錯誤發生在 Database_write 函式裡呼叫 rewind() 這個處理 FILE 類內建函式把 steam 指向的位置重置為開頭時發生錯誤,但想好久還是不懂為什麼會錯,難道是 conn->file 這邊由 -> 讀取 file 時產生問題?
再仔細檢查,發現是自己打錯了!

conn->db = malloc(sizeof(struct Database) + sizeof(char[max_rows]));

應該改成

conn->db = malloc(sizeof(struct Database) + sizeof(struct Address[max_rows]));

由於參考 C99 規格書 在 6.7.2.1.16 FAM(Flexible Array Members) 時忘記改為正確的類型。
這讓我好奇,先回憶 struct Connection

struct Connection {
    FILE *file;
    struct Database *db;
};

FILE *filestruct Database *db
為什麼在 struct Connection conn 的成員 db 呼叫 malloc ,分配空間時輸入不對的類別,會造成後來使用 rewind 讀取 conn 的成員 file 時發生找不到的錯誤?

後來找到解答,由於前面分配錯的空間( + sizeof(char[max_rows]),這樣是分配 max_rowschar 的空間,但是再回看 struct Database :

struct Database {
    int max_rows;
    struct Address rows[];
}

這邊rows是 struct Address type , 所以儘管分配錯誤的空間大小 ( + sizeof(char[max_rows])) ,但是當存取 rows[i] 對應的 element 的時候,它會跳 i 個 struct Address type, 所以整個記憶體位置就不對了。
而實際看看問題出在哪裡:

./ex17_ext2 db.dat c

第一個參數 db.dat 代表一個檔案名稱,第二個參數 c 是產生第一個檔案,裡面存 MAX_ROWSstruct Address,而這兩個參數在執行過程會依序呼叫 Database_openDatabase_createDatabase_write。而錯誤發生在 Database_write裡面的 rewind() ,經過 gdb 執行過程找到問題,是這樣產生的:

  1. Database_open ,先 malloc 分配出可用記憶體空間給 conn , 之後 malloc 分配空間給 conn->db , 再來 fopen 建立 conn->file,
    gdb 裡顯示 connconn->dbconn->file 的記憶體位置,原先能正常執行時各個變數記憶體位置可能如下:
1: conn = (struct Connection *) 0x55555632a0
2: conn->db = (struct Database *) 0x55555632c0
3: conn->file = (FILE *) 0x555557c5f0

而分配錯誤空間後各個變數記憶體位置可能如下:

1: conn = (struct Connection *) 0x55555632a0
2: conn->db = (struct Database *) 0x55555632c0
3: conn->file = (FILE *) 0x55555632e0

可以看出前者 conn->file 的位置跟 conn->db 兩個變數的 差值 比 後者 兩個變數的差值還大,先是由於分配錯誤空間大小導致,這邊還沒有問題,問題發生在之後 conn->create 的時候用新產生的 struct Address assign 給 rows的時候,由此把 conn->file 的內容也改寫掉導致。
2. 由此也可觀察出 structelement 的位置其記憶體位址不一定從上到下,回看用 malloc 建立 conn->db 再建立 conn->file 可以看出 後者的記憶體位置是接在 conn->db 之後而不是之前。
最後放上能運行的:
ex17_ext2.c

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

#define MAX_DATA 512
#define MAX_ROWS 100

struct Address {
    int id;
    int set;
    char name[MAX_DATA];
    char email[MAX_DATA];
};

struct Database {
    int max_rows;
    struct Address rows[];
};

struct Connection {
    FILE *file;
    struct Database *db;
};

void Database_close(struct Connection *conn);

void die(const char *message, struct Connection *conn)
{
    if (errno) {
        if (conn) {
            Database_close(conn);
        }
        perror(message);
    } else {
        printf("ERROR: %s\n", message);
        if (conn) {
            Database_close(conn);
        }
    }

    exit(1);
}

void Address_print(struct Address *addr)
{
    printf("%d %s %s\n", addr->id, addr->name, addr->email);
}

void Database_load(struct Connection *conn, int max_rows)
{
    int rc = fread(conn->db, sizeof(struct Database) + sizeof(struct Address[max_rows]), 1, conn->file);
    if (rc != 1)
        die("Failed to load database.", conn);
}

struct Connection *Database_open(const char *filename, char mode, int max_rows)
{
    struct Connection *conn = malloc(sizeof(struct Connection));
    if (!conn)
        die("Memory error", conn);

    conn->db = malloc(sizeof(struct Database) + sizeof(struct Address[max_rows]));
    if (!conn->db)
        die("Memory error", conn);

    conn->db->max_rows = max_rows;

    if (mode == 'c') {
        conn->file = fopen(filename, "w");
    } else {
        conn->file = fopen(filename, "r+");

        if (conn->file) {
            Database_load(conn, max_rows);
        }
    }

    if (!conn->file)
        die("Failed to open the file", conn);

    return conn;
}

void Database_close(struct Connection *conn)
{
    if (conn) {
        if (conn->file)
            fclose(conn->file);
        if (conn->db)
            free(conn->db);
        free(conn);
    }
}

void Database_write(struct Connection *conn, int max_rows)
{
    rewind(conn->file);

    int rc = fwrite(conn->db, sizeof(struct Database) + sizeof(struct Address[max_rows]), 1, conn->file);
    if (rc != 1)
        die("Failed to write database.", conn);

    rc = fflush(conn->file);
    if (rc == -1)
        die("Cannot flush database.", conn);
}

void Database_create(struct Connection *conn, int max_rows)
{
    int i = 0;

    for (i = 0; i < max_rows; i++) {
        // make a prototype to initialize it
        struct Address addr = {.id = i, .set = 0};

        // then just assign it
        conn->db->rows[i] = addr;
    }
}

void Database_set(struct Connection *conn, int id, const char *name,
        const char *email)
{
    struct Address *addr = &conn->db->rows[id];
    if (addr->set)
        die("Already set, delete it first", conn);

    addr->set = 1;
    // WARNING: bug, read the "How To Break It" and fix this
    char *res = strncpy(addr->name, name, MAX_DATA);
    // bug fixed 
    addr->name[MAX_DATA - 1] = '\0';
    // demonstrate the strncpy bug
    if (!res)
        die("Name copy failed", conn);

    res = strncpy(addr->email, email, MAX_DATA);
    if (!res)
        die("Email copy failed", conn);
    // bug fixed
    addr->email[MAX_DATA - 1] = '\0';
}

void Database_get(struct Connection *conn, int id)
{
    struct Address *addr = &conn->db->rows[id];

    if (addr->set) {
        Address_print(addr);
    } else {
        die("ID is not set", conn);
    }
}

void Database_delete(struct Connection *conn, int id)
{
    struct Address addr = {.id = id, .set = 0};
    conn->db->rows[id] = addr;
}

void Database_list(struct Connection *conn, int max_rows)
{
    int i = 0;
    struct Database *db = conn->db;

    for (i = 0; i < max_rows; i++) {
        struct Address *cur = &db->rows[i];

        if (cur->set) {
            Address_print(cur);
        }
    }
}

int main(int argc, char *argv[])
{
    if (argc < 3)
        die("USAGE: ex17 <dbfile> <action> [action params]", NULL);

    char *filename = argv[1];
    char action = argv[2][0];

    // adding this to setting max_rows
    int max_rows = 10;
    struct Connection *conn = Database_open(filename, action, max_rows);
    int id = 0;

    if (argc > 3) id = atoi(argv[3]);
    if (id >= MAX_ROWS) die("There's not that many record.", conn);

    switch(action) {
        case 'c':
            Database_create(conn, max_rows);
            Database_write(conn, max_rows);
            break;

        case 'g':
            if (argc != 4)
                die("Need an id to get", conn);

            Database_get(conn, id);
            break;

        case 's':
            if (argc != 6)
                die("Need id, name, email to set", conn);

            Database_set(conn, id, argv[4], argv[5]);
            Database_write(conn, max_rows);
            break;

        case 'd':
            if (argc != 4)
                die("Need id to delete", conn);

            Database_delete(conn, id);
            Database_write(conn, max_rows);
            break;

        case 'l':
            Database_list(conn, max_rows);
            break;
        default:
            die("Invalid action: c=create, g=get, s=set, d=del, l=list", conn);
    }

    Database_close(conn);

    return 0;
}