Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rt_db_put_internal() does not work with inmem database #161

Open
bylee20 opened this issue Sep 19, 2024 · 3 comments
Open

rt_db_put_internal() does not work with inmem database #161

bylee20 opened this issue Sep 19, 2024 · 3 comments

Comments

@bylee20
Copy link

bylee20 commented Sep 19, 2024

I have created empty database with db_create_inmem() and It seems that rt_db_put_internal() does not work with inmem database.
Here's my test code:

    const auto db = db_create_inmem();

    rt_ell_internal *sph = nullptr;
    BU_ALLOC(sph, rt_ell_internal);
    *sph = {
        .magic = RT_ELL_INTERNAL_MAGIC,
        .v = {0, 0, 0},
        .a = {1, 0, 0},
        .b = {0, 1, 0},
        .c = {0, 0, 1},
    };

    rt_db_internal in = {
        .idb_magic = RT_DB_INTERNAL_MAGIC,
        .idb_major_type = DB5_MAJORTYPE_BRLCAD,
        .idb_minor_type = ID_SPH,
        .idb_meth = &OBJ[ID_SPH],
        .idb_ptr = sph,
        .idb_avs = BU_AVS_INIT_ZERO,
    };

    const auto dir = db_diradd(db, "sph.o", RT_DIR_PHONY_ADDR, 0, RT_DIR_SOLID, &in.idb_minor_type);
    rt_db_put_internal(dir, db, &in, &rt_uniresource);

If you execute this code, you will see error message:

rt_db_put_internal5(sph.o) db_realloc5() failed

I did short grepping and found that this error comes from:

brlcad/src/librt/db5_io.c

Lines 767 to 773 in 02ba341

/* Second, obtain storage for final object */
if (ep->ext_nbytes != dp->d_len || (size_t)dp->d_addr == (size_t)RT_DIR_PHONY_ADDR) {
if (db5_realloc(dbip, dp, ep) < 0) {
bu_log("db_put_external(%s) db_realloc5() failed\n", dp->d_namep);
return -5;
}
}

which is result of failure of db5_realloc() call and again, this failure is originated from:

/* make sure the database directory is initialized */
if (dbip->dbi_eof == RT_DIR_PHONY_ADDR) {
int ret = db_dirbuild(dbip);
if (ret) {
return -1;
}
}

Here, db_dirbuild() call always fails for inmem database because of this check:

brlcad/src/librt/db5_scan.c

Lines 387 to 389 in 02ba341

if (!dbip->dbi_fp) {
return -1;
}

Since dbip is for inmem database, the file pointer will be always null and db_dirbuild() never work.

I also found that wdb_put_internal() works fine with inmem database.

Is this by design or a bug? Or, did I do something wrong?

@starseeker
Copy link
Member

@brlcad do you have any insights about the design intent here? Is rt_db_put_internal expected to work in this case?

@bylee20
Copy link
Author

bylee20 commented Sep 30, 2024

@brlcad do you have any insights about the design intent here? Is rt_db_put_internal expected to work in this case?

Thank you for your attention.

I think most people would expect it will work unless it is explicitly documented, since... I mean, literally, why not?

As a matter of fact, I found this while using libged with inmem db.
It seems that libged can work with inmem db by giving "inmem" and the address of db_i pointer as arguments.
However, as soon as I attempted to make a geometry with "make" command, this failed because of this.
As I mentioned in the issue, I can use wdb_put_internal() but this is not possible when interop with libged because the rt_db_put_internal() call is done in libged side.

@brlcad
Copy link
Member

brlcad commented Sep 30, 2024

@bylee20 it's basically a bug. As you note, it could/can work, so why not. It should work. The practical issue is just that all the API code paths (and there's dozens of paths for this particular one) for reading and writing in-mem databases were narrowly developed for specific features, so more work is needed in other areas that are not yet exercised by an end-user feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants