malloc指针地址改变导致段错误？

Question

malloc指针地址改变导致段错误？

drd*_*dot 2 c pointers segmentation-fault

我尝试使用C编写一个简单的数据库.但是,我尝试调试我的分段错误,并发现通过malloc获得的内存指针似乎正在改变(名称和电子邮件指针似乎指向Database_load程序执行之前和之后的不同内存位置).我有两个问题:

为什么在执行Database_load之前和之后内存指针(名称和电子邮件)指向不同的位置？
程序为什么会产生seg错误？

这是与问题相关的代码

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

int MAX_DATA;
int MAX_ROWS;

struct Address {
    int id;
    int set;
    //int MAX_DATA;
    char *name;
    char *email;
};

struct Database {
    //int MAX_ROWS;
    struct Address *rows;
};

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

void die(const char *message){
    if(errno){
        //perror(message);
        printf("ERROR: %s\n", 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 i;
    int rc = fread(conn->db, sizeof(struct Database), 1, conn->file);
    if(rc != 1) die("Failed to load database.");

    for (i = 0; i < MAX_ROWS; i++) {
    printf("test Database_load loop read rows %p\n", &conn->db->rows[i]);   
    printf("test Database_load loop read rows name %p\n", &conn->db->rows[i].name); 
    printf("test Database_load loop read rows email %p\n", &conn->db->rows[i].email);   
    printf("test Database_load loop read rows name %s\n", conn->db->rows[i].name);  
    printf("test Database_load loop start %d\n", i);    
        rc = fread(&conn->db->rows[i], sizeof(struct Address), 1, conn->file);
    printf("test Database_load loop read rows %d\n", i);    
      rc = fread(&conn->db->rows[i].name, sizeof(MAX_DATA), 1, conn->file);
    printf("test Database_load loop read name %d\n", i);    
      rc = fread(&conn->db->rows[i].email, sizeof(MAX_DATA), 1, conn->file);
    printf("test Database_load loop read email %d\n", i);   
      if(rc != 1) die("Failed to load database.");
    printf("test Database_load loop\n");    
    }
}

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

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

    conn->db->rows = malloc(sizeof(struct Address) * MAX_ROWS);
    if (conn->db->rows == NULL) die("No memory for rows");
    for(i = 0; i < MAX_ROWS; i++){
        // make a prototype to initialize it
        //struct Address addr = {.id = i, .set = 0};
        conn->db->rows[i].id = i;
        conn->db->rows[i].set = 0;
        conn->db->rows[i].name = malloc(sizeof(char) * MAX_DATA);
      if (conn->db->rows[i].name == NULL) die("No memory for name");
        conn->db->rows[i].email = malloc(sizeof(char) * MAX_DATA);
      if (conn->db->rows[i].email == NULL) die("No memory for email");
        // then just assign it
        if (i == 0) {
      printf("test set name = %p\n", &conn->db->rows[i].name);
      printf("test set email = %p\n", &conn->db->rows[i].email);
        }
    }

    if(mode == 'c'){
        conn->file = fopen(filename, "w");
    }
    else{
        conn->file = fopen(filename, "r+"); //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){
    int i = 0;
    rewind(conn->file);
    int rc = fwrite(conn->db, sizeof(struct Database), 1, conn->file);
    if(rc != 1) die("Failed to write database.");
    for (i = 0; i < MAX_ROWS; i++) {
        rc = fwrite(&conn->db->rows[i], sizeof(struct Address), 1, conn->file);

        if(rc != 1) die("Failed to write database.");
        rc = fwrite(&conn->db->rows[i].name, sizeof(MAX_DATA), 1, conn->file);
        if(rc != 1) die("Failed to write database.");
        rc = fwrite(&conn->db->rows[i].email, sizeof(MAX_DATA), 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 MAX_DATA, int MAX_ROWS){
    int i = 0;
    conn->db->rows = malloc(sizeof(struct Address) * MAX_ROWS);
    if (conn->db->rows == NULL) die("No memory for rows");
    for(i = 0; i < MAX_ROWS; i++){
        // make a prototype to initialize it
        struct Address addr = {.id = i, .set = 0};
        addr.name = malloc(sizeof(char) * MAX_DATA);
      if (addr.name == NULL) die("No memory for name");
        addr.email = malloc(sizeof(char) * MAX_DATA);
      if (addr.email == NULL) die("No memory for email");
        // 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: intentional bug, no relevant this question
    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> <MAX_ROWS> <MAX_DATA> [action params]");
    char *filename = argv[1];
    char action = argv[2][0];
    MAX_DATA = atoi(argv[3]);
    MAX_ROWS = atoi(argv[4]);

    int id = 0;
  if(argc > 5) id = atoi(argv[5]);
    struct Connection *conn = Database_open(filename, action);

    // legacy code, does not apply for create case
//  if(argc > 3) id = atoi(argv[3]);
//  if(id >= MAX_ROWS) die("There's not that many records.");

    switch(action){
        case 'c':
            if(argc != 5) die("Need MAX_DATA and MAX_ROWS");
            Database_create(conn, MAX_DATA, MAX_ROWS);
            Database_write(conn);
            break;

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

        case 's':
            if(argc != 8) die("Need id, name, email to set");
            Database_set(conn, id, argv[6], argv[7]);
            Database_write(conn);
            break;

        case 'd':
            if(argc != 6) die("Need id to delete");
            Database_delete(conn, id);
            Database_write(conn);
            break;

        case 'l':
            Database_list(conn);
            break;

        default:
            die("Invalid action, only: c=create, g=get, s=set, d=del, l=list");
    }

    Database_close(conn);

    return 0;
}

Run Code Online (Sandbox Code Playgroud)

这是我执行程序后的printf输出

$./ex17_arbitrary db_arbitrary.dat c 512 100
$./ex17_arbitrary db_arbitrary.dat s 512 100 1 zed zed@zedshaw.com

test set name = 0x15ad058
test set email = 0x15ad060
test Database_load loop read rows (nil)
test Database_load loop read rows name 0x8
test Database_load loop read rows email 0x10

Run Code Online (Sandbox Code Playgroud)

我注意到的一件事是,这两行在使用相同命令的多次执行中永远不会改变

test Database_load loop read rows name 0x8
test Database_load loop read rows email 0x10

Run Code Online (Sandbox Code Playgroud)

更新:我还有一些额外的设计问题.看起来当前数据结构的设计存在问题.我将在这里详细说明设计要求:

除了我创建的功能之外,我不需要任何额外的功能.数据库的大小(MAX_DATA和MAX_ROWS必须是可变的).现在我每次调用程序时都会输入MAX_DATA和MAX_ROWS.这可以改善吗？我想可能只是在需要使用Database_create方法时给出MAX_DATA和MAX_ROWS.这个程序来自一个有趣的练习(c.learncodethehardway.org/book/ex17.html),原始程序有一个修复大小的数据库.目标是使其变成可变大小.

Answer 1

Cra*_*tey 5

在Database_load,你正在做:

int rc = fread(conn->db, sizeof(struct Database), 1, conn->file);

Run Code Online (Sandbox Code Playgroud)

但是,conn->db是一个指向类型的指针,struct Database唯一的元素是:

struct Address *rows;

Run Code Online (Sandbox Code Playgroud)

简而言之,您正在尝试从文件的一个位置初始化指针 .这不是指向指针变量本身的内容的缓冲区/数组(即内存中指向的地址)rowsfreadrowsrowsrows

既然你已经初始化rows了Database_open,那么fread看起来很可疑,因为:

你已经设置好了 rows
做(例如)void *ptr = ...; read(fd,&ptr,sizeof(ptr));几乎从不正确.
更正常的用法是: void *ptr = ...; read(fd,ptr,some_length);

您通过执行上述(2)的等效来覆盖[废弃] rows初始化的值Database_open.如果你写的话,它就像[坏]一样:

conn->db->rows = NULL;

Run Code Online (Sandbox Code Playgroud)

要么:

conn->db->rows = (void *) 0x1234;

Run Code Online (Sandbox Code Playgroud)

我不完全确定,因为我无法在没有数据的情况下测试程序,但您可以简单地删除上面的内容fread.或者,如果数据库中真正存在某种类型的标题,那么它必须替换为其他类型的标题.

但是,如果你拿出fread来,rows保持完整,它指向的内容将for像现在一样填充在循环中.

更新:

我看到了问题.我认为这是一个更糟糕的设计.基本上,我将指针存储到数据库中并尝试将其读出并在不同的程序执行中访问相同的指针地址.

我在我的原始帖子中提到过,但是在我的编辑中删除了它,因为我认为你没有尝试这样做而且fread更像是一个"错字".

但是,尝试在文件中存储指针的持久值并在下次调用时恢复它们是一种糟糕的设计.

特别是如果指针来自malloc.旧指针可能会malloc在第二次调用时发生冲突.而且,你怎么能告诉malloc老指针现在以某种方式"保留"？

对于指向全局/静态内存的指针,如果重建程序并添加新变量[更改所有内容的地址和偏移量]会发生什么？

简而言之,你不能这样做[这也是很长的答案:-)].

我能想到的解决方案是只存储struct Address,并命名字符串,地址字符串.这会是一个更好的设计吗？

是的,如果你的意思是:

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

Run Code Online (Sandbox Code Playgroud)

现在,struct Address可以读/写一个文件,因为它没有指针(即那是键)

作为另一个例子,考虑如果struct Address有嵌入式链表列表指针会发生什么:

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

// simple traversal
for (addr = addrlist;  addr != NULL;  addr = addr->next) {
    // do stuff ...

    // broken, because struct Address has a pointer
    fwrite(addr,sizeof(struct Address),fout);
}

// fix for above
for (addr = addrlist;  addr != NULL;  addr = addr->next) {
    // do stuff ...

    fwrite(addr->id,sizeof(addr->id),fout);
    fwrite(addr->set,sizeof(addr->set),fout);
    fwrite(addr->name,sizeof(addr->name),fout);
    fwrite(addr->email,sizeof(addr->email),fout);
}

Run Code Online (Sandbox Code Playgroud)

这是一个更孤立的方式来做列表:

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

// NOTE: either of these works
#if 1
struct AddrLink {
    struct AddrLink *link;
    struct Address *data;
};

#else
struct AddrLink {
    struct AddrLink *link;
    struct Address data;
};
#endif

// indirect list traversal
for (link = addrlist;  link != NULL;  link = link->next) {
    // do stuff ...

    // works because struct Address does _not_ have a pointer
    fwrite(link->data,sizeof(struct Address),fout);
}

Run Code Online (Sandbox Code Playgroud)

在一般情况下,您要做的与序列化类似.这是一个链接:C - 序列化技术这是另一个:用C语言序列化数据结构

当我这样做时,我喜欢在数据前加上标准的"section"标题.类似的技术被使用.mp4,.avi文件:

struct section {
    int type;                           // section type
    int totlen;                         // total section length
    int size;                           // sizeof of section element
    int count;                          // number of array elements
};

#define TYPE_ADDRESS    1
#define TYPE_CITY       2
#define TYPE_STATE      3
#define TYPE_COUNTRY    4

Run Code Online (Sandbox Code Playgroud)

这样,如果你的程序不理解新类型,因为它是较旧的版本,它仍然可以复制或跳过它不理解的数据而不会损害它.(例如)在处理.mp4文件时这是必需的行为.

更新#2:

我已经发布了完整的代码.你能建议一个更好的设计方法吗？我没有对数据库文件格式化的特定约束

好的,下面的工作代码......

我用结构改变了一些东西[并重命名].值得注意的是,您调用的主结构[ Connection现在称为database_t].Address现在address_t.

你非常接近.你试图用你的旧东西 做什么struct Database,我换了dbheader_t.也就是说,这些是我正在谈论的数据库头结构.你的指针就在里面.我在行数据开始之前将最大行数和最大数据记录为数据库文件的第一部分

我将分配代码移动到一个新函数Database_alloc[因为它现在必须在两个不同的地方调用].

Database_open必须要稍微聪明一些.对于c操作,它会填充数据库标头.对于所有其他操作,它必须打开DB文件并读取磁盘头.

此外,conn->db->rows现在使用新的结构组织,而不是随处可见db->rows.

总的来说,你已经非常接近了.

我也重新设计main更有点人性化(即你只需要输入MAX_DATA/MAX_ROWS的c命令.

无论如何,这是[请原谅无偿的风格清理]:

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

// database element
typedef struct _address {
    int id;                             // ID/slot number
    int set;                            // 1=active, 0=free/available
    char *name;                         // person's name
    char *email;                        // person's email address
} address_t;

// database on-disk header
typedef struct _header {
    int max_rows;                       // maximum number of rows
    int max_data;                       // maximum size of a field
} dbheader_t;
// NOTE: other stuff can be added (e.g. split max_data into max_name and
// max_email so that each field can have its own maximum length)

// database control
typedef struct _database {
    FILE *file;                         // database I/O stream
    dbheader_t header;                  // copy of on-disk header
    address_t *rows;                    // database data
} database_t;

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

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

void
Database_load(database_t *db)
{
    int i;
    address_t *addr;
    int rc;

    // NOTE: database header has _already_ been read
#if 0
    rc = fread(db->file, sizeof(dbheader_t), 1, db->file);
    if (rc != 1)
        die("Failed to write database.");
#endif

    for (i = 0; i < db->header.max_rows; i++) {
        addr = &db->rows[i];

        rc = fread(&addr->id, sizeof(addr->id), 1, db->file);
        if (rc != 1)
            die("Failed to write database.");

        rc = fread(&addr->set, sizeof(addr->set), 1, db->file);
        if (rc != 1)
            die("Failed to write database.");

        rc = fread(addr->name, db->header.max_data, 1, db->file);
        if (rc != 1)
            die("Failed to write database.");

        rc = fread(addr->email, db->header.max_data, 1, db->file);
        if (rc != 1)
            die("Failed to write database.");
    }
}

void
Database_alloc(database_t *db)
{
    address_t *addr;

    db->rows = malloc(sizeof(address_t) * db->header.max_rows);
    if (db->rows == NULL)
        die("No memory for rows");

    for (int i = 0; i < db->header.max_rows; i++) {
        addr = &db->rows[i];

        // NOTE: no need to do it this way
        // make a prototype to initialize it
        // struct Address addr = {.id = i, .set = 0};

        addr->id = i;
        addr->set = 0;

        addr->name = calloc(db->header.max_data,sizeof(char));
        if (addr->name == NULL)
            die("No memory for name");

        addr->email = calloc(db->header.max_data,sizeof(char));
        if (addr->email == NULL)
            die("No memory for email");
    }
}

database_t *
Database_open(const char *filename, char mode, int max_rows, int max_data)
{
    int rc;

    database_t *db = calloc(1,sizeof(database_t));
    if (!db)
        die("Memory error no db pointer");

    switch (mode) {
    case 'c':
        db->file = fopen(filename, "w");
        if (!db->file)
            die("Failed to open the file");

        // set up a header [to write out]
        db->header.max_rows = max_rows;
        db->header.max_data = max_data;

        Database_alloc(db);
        break;

    default:
        db->file = fopen(filename, "r+");   // r+?
        if (!db->file)
            die("Failed to open the file");

        // read in header so we know the number of rows and the max data size
        rc = fread(&db->header,sizeof(dbheader_t),1,db->file);
        if (rc != 1)
            die("Failed to read header.");

        Database_alloc(db);
        Database_load(db);
    }

    return db;
}

void
Database_close(database_t *db)
{
    address_t *addr;

    if (db) {
        if (db->file)
            fclose(db->file);
        db->file = NULL;

        if (db->rows) {
            for (int rowidx = 0;  rowidx < db->header.max_rows;  ++rowidx) {
                addr = &db->rows[rowidx];
                free(addr->name);
                free(addr->email);
            }
            free(db->rows);
            db->rows = NULL;
        }

        free(db);
    }
}

void
Database_write(database_t *db)
{
    int i;
    int rc;
    address_t *addr;

    rewind(db->file);

    // write out the DB header
    rc = fwrite(&db->header, sizeof(dbheader_t), 1, db->file);
    if (rc != 1)
        die("Failed to write database.");

    for (i = 0; i < db->header.max_rows; i++) {
        addr = &db->rows[i];

        rc = fwrite(&addr->id, sizeof(addr->id), 1, db->file);
        if (rc != 1)
            die("Failed to write database.");

        rc = fwrite(&addr->set, sizeof(addr->set), 1, db->file);
        if (rc != 1)
            die("Failed to write database.");

        rc = fwrite(addr->name, db->header.max_data, 1, db->file);
        if (rc != 1)
            die("Failed to write database.");

        rc = fwrite(addr->email, db->header.max_data, 1, db->file);
        if (rc != 1)
            die("Failed to write database.");
    }

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

void
Database_set(database_t *db, int id, const char *name, const char *email)
{
    address_t *addr = &db->rows[id];

    if (addr->set)
        die("Already set, delete it first");
    addr->set = 1;

    // warning: intentional bug, no relevant this question
    // demonstrate the strncpy bug
    char *res = strncpy(addr->name, name, db->header.max_data);
    if (!res)
        die("Name copy failed");
    addr->name[db->header.max_data - 1] = 0;

    res = strncpy(addr->email, email, db->header.max_data);
    if (!res)
        die("Email copy failed");
    addr->email[db->header.max_data - 1] = 0;
}

void
Database_get(database_t *db, int id)
{
    address_t *addr = &db->rows[id];

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

void
Database_delete(database_t *db, int id)
{

    // NOTE/BUG: this causes a memory leak because it overwrites the name and
    // email fields without freeing them first
#if 0
    struct Address addr = {.id = id,.set = 0 };
    db->rows[id] = addr;
#else
    address_t *addr = &db->rows[id];
    addr->id = 0;
    addr->set = 0;
    memset(addr->name,0,db->header.max_data);
    memset(addr->email,0,db->header.max_data);
#endif
}

void
Database_list(database_t *db)
{
    int i;

    for (i = 0; i < db->header.max_rows; i++) {
        address_t *cur = &db->rows[i];
        if (cur->set) {
            Address_print(cur);
        }
    }
}

int
main(int argc, char *argv[])
{
    int max_data = 0;
    int max_rows = 0;
    int id = -1;

    if (argc < 3) {
        printf("USAGE: ex17 <dbfile> <action> [action params]");
        printf("  actions:\n");
        printf("    c <MAX_DATA> <MAX_ROWS> -- create database\n");
        printf("    g <id> -- get id and print\n");
        printf("    s <id> <name> <email> -- set id\n");
        printf("    d <id> -- delete id\n");
        printf("    l -- list database\n");
        die("aborting");
    }

    // skip over program name
    --argc;
    ++argv;

    --argc;
    char *filename = *argv++;

    --argc;
    char action = argv[0][0];
    ++argv;

    switch (action) {
    case 'c':
        if (argc != 2)
            die("Need MAX_DATA and MAX_ROWS");
        max_data = atoi(argv[0]);
        max_rows = atoi(argv[1]);
        break;
    }

    database_t *db = Database_open(filename, action, max_rows, max_data);

    // legacy code, does not apply for create case
//  if(argc > 3) id = atoi(argv[3]);
//  if(id >= db->header.max_rows) die("There's not that many records.");

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

    case 'g':
        if (argc != 1)
            die("Need an id to get");
        id = atoi(argv[0]);
        Database_get(db, id);
        break;

    case 's':
        if (argc != 3)
            die("Need id, name, email to set");
        id = atoi(argv[0]);
        Database_set(db, id, argv[1], argv[2]);
        Database_write(db);
        break;

    case 'd':
        if (argc != 1)
            die("Need id to delete");
        id = atoi(argv[0]);
        Database_delete(db, id);
        Database_write(db);
        break;

    case 'l':
        Database_list(db);
        break;

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

    Database_close(db);

    return 0;
}

Run Code Online (Sandbox Code Playgroud)

归档时间：	9 年，10 月前
查看次数：	210 次
最近记录：	9 年，10 月前