nasa / bplib

Apache License 2.0
27 stars 13 forks source link

Use of variably-sized structures #33

Closed jphickey closed 4 years ago

jphickey commented 4 years ago

There are two structure types that are variably sized: bp_object_t in bplib.h and bp_payload_data_t in bundle_types.h. Furthermore, the bp_payload_data_t is used as a member within the bp_payload_t structure, which is invalid.

Recommendation is to split the "fixed" fields into a separate header structure, i.e. everything prior to the variably-sized payload/data element should be its own struct, which can be used when only the header fields are relevant.

Also recommend use of a dedicated allocator function for variably-sized objects to be sure it is always malloc'ed correctly and consistently for the required size, and to perform any related error checking to make sure it isn't too small or too big. For instance when dequeuing from a file backend it will malloc() whatever size it read from the file, which could be zero or up to 4GB.

jphickey commented 4 years ago

To clarify: the bp_object_t is currently:

typedef struct {
    int         handle;
    bp_sid_t    sid;
    int         size;
    char        data[];
} bp_object_t;

Suggest to do something like:

typedef struct {
    int         handle;
    bp_sid_t    sid;
    int         size;
} bp_object_header_t;

typedef struct {
    bp_object_header_t header; 
    char        data[];
} bp_object_t;

Then, for functions that really only need the header and/or the payload data is separate, they should be passed a bp_object_header_t, rather than a bp_object_t. Likewise, the size of the header is readily available as sizeof(bp_object_header_t) rather than offsetof(bp_object_t, data) which is rather complicated to do all the time.

jpswinski commented 4 years ago

Fixed - see above commits.