Slightly clearer description for the size of dpg_rpt field on Data Page documentation

[araujoarthur]

Fixed an open backtick on hdr_bumped_transaction description.

Made a slightly clearer description for the size of the dpg_rpt field that now explicitly accounts for what is shown in the code block above it: An array of structs with two two byte fields.

This PR contains a change I'm not sure about: I changed the starting offset of dpg_rpt. I tested I structure like the one described and got the following results:

 BEGINOF Test of Offsets for the DATA_PG structure

Offset of Header:         0x0000000000000000  | Limit of Header:         0x0000000000000010   | Size of Header:        16
Offset of dpg_sequence:   0x0000000000000010  | Limit of dpg_sequence:   0x0000000000000014   | Size of dpg_sequence:  4
Offset of dpg_relation:   0x0000000000000014  | Limit of dpg_relation:   0x0000000000000016   | Size of dpg_relation:  2
Offset of dpg_count:      0x0000000000000016  | Limit of dpg_count:      0x0000000000000018   | Size of dpg_count:     2
Offset of dpg_rpt:        0x0000000000000018  | Limit of dpg_rpt:        0x000000000000001c   | Size of dpg_rpt:       4
        Offset of dpg_rpt.dpg_offset:        0x0000000000000018  | Limit of dpg_rpt.dpg_offset:        0x000000000000001a   | Size of dpg_offset:       2
        Offset of dpg_rpt.dpg_length:        0x000000000000001a  | Limit of dpg_rpt.dpg_length:        0x000000000000001c   | Size of dpg_length:       2

Total Size of Struct: 8
 ENDOF Test of Offsets for the DATA_PG structure

But it might be a convention that within the file it's padded by 2 bytes, I can't really verify right now.

I also would like to bring the suggestion to change the struct representation where arrays are present as last element from

struct pointer_page
{
    pag ppg_header;
    SLONG ppg_sequence;
    SLONG ppg_next;
    USHORT ppg_count;
    USHORT ppg_relation;
    USHORT ppg_min_space;
    USHORT ppg_max_space;
    SLONG ppg_page[1];
};

to

struct pointer_page
{
    pag ppg_header;
    SLONG ppg_sequence;
    SLONG ppg_next;
    USHORT ppg_count;
    USHORT ppg_relation;
    USHORT ppg_min_space;
    USHORT ppg_max_space;
    SLONG ppg_page[];
};

using Flexible Array Members to make it easier to fit data into the struct, unless there's a better reason to use SLONG ppg_page[1] I don't know yet.

[mrotteveel]

Which ODS version did you check? Newer Firebird versions can have a different layout than that described in the document.

[araujoarthur]

The test result I sent was not from a fdb file. I wrote the struct in C, packed it and tested the offset of each member. Bellow I'm attaching a screenshot of the data paga (starting at 0x14000) for an actual file, ODS 0x800b, where the dpg_offset (light green) starts at 0x14018.

[mrotteveel]

Indeed, it looks like a miscount.

[mrotteveel]

I also would like to bring the suggestion to change the struct representation where arrays are present as last element from

[..]

to
[..]

using Flexible Array Members to make it easier to fit data into the struct, unless there's a better reason to use SLONG ppg_page[1] I don't know yet.

The structs are copied from their definition in the Firebird sources, e.g. see src/jrd/ods.h of Firebird 2.1 on which the current document is based, or src/jrd/ods.h on master for the latest version. Given the Firebird sources use the same definition, I don't think it should change here.

[mrotteveel]

Small note: I notice you submit your changes from the master of your fork. You may want to consider creating a branch for your changes, and leaving your master clean so it is easier to sync with our repository.

[mrotteveel]

As to the miscount, it seems I didn't pay attention when reviewing #161

[araujoarthur]

Small note: I notice you submit your changes from the master of your fork. You may want to consider creating a branch for your changes, and leaving your master clean so it is easier to sync with our repository.

Yeah, I can totally do that, thanks for the tip! You mean create a branch for the changes and merge them to the master after finished or submit the PR directly from the branch?

I also would like to bring the suggestion to change the struct representation where arrays are present as last element from

[..]

to
[..]
using Flexible Array Members to make it easier to fit data into the struct, unless there's a better reason to use SLONG ppg_page[1] I don't know yet.

The structs are copied from their definition in the Firebird sources, e.g. see src/jrd/ods.h of Firebird 2.1 on which the current document is based, or src/jrd/ods.h on master for the latest version. Given the Firebird sources use the same definition, I don't think it should change here.

Oh! Okay, if it's the case, I agree. I'll check their source. At first glance I thought writing this way would lead to undefined behavior, but I'm likely wrong. Thanks 😄

[mrotteveel]

Small note: I notice you submit your changes from the master of your fork. You may want to consider creating a branch for your changes, and leaving your master clean so it is easier to sync with our repository.

Yeah, I can totally do that, thanks for the tip! You mean create a branch for the changes and merge them to the master after finished or submit the PR directly from the branch?

You should submit the PR from the branch.

Oh! Okay, if it's the case, I agree. I'll check their source. At first glance I thought writing this way would lead to undefined behavior, but I'm likely wrong. Thanks 😄

Not sure of the details of that. Consider asking on firebird-devel of you're interested.