Wrong-size allocation in row.cc

[Wim Lewis]

row.cc [ http://bazaar.launchpad.net/~drizzle-trunk/libdrizzle/libdrizzle-redux/view/99.1.7/libdrizzle/row.cc#L109 ] contains this odd line of code:

    result->row= new (std::nothrow) drizzle_field_t[sizeof(size_t) * result->column_count];

which allocates some memory for the row, but the amount of memory allocated is wrong. The buffer will eventually contain one drizzle_field_t followed by a number of size_ts, but this code allocates enough space for a number of drizzle_field_ts equal to the number of bytes in the desired array of size_ts ... which will practically always be enough, and usually far more than needed.

Probably the most straightforward fix is to switch to malloc/free for that heterogeneous buffer and either malloc(sizeof(drizzle_field_t) + sizeof(size_t) * result->column_count), or if you want to avoid as many pointer casts/arithmetic and also be portable to machines with exotic memory architectures, define a structure

struct row_info {
    drizzle_field_t row;
    size_t sizes[1];
}

make result->row be a pointer to this structure, and malloc(sizeof(struct row_info) - sizeof(size_t) + sizeof(size_t) * result->column_count).

[Andrew Hutchings (linuxjedi)]

I'd rather not switch it back to malloc/free. We recently moved away from that where practical. The struct sounds safer/better to me

[Wim Lewis (wiml-omni)]

I don't use C++ much, but I don't think you can do it with 'new' (other than just allocating a char array and then casting pointers like you would with malloc, in which case why not use malloc?). Last I checked C++ doesn't really like heterogeneous arrays or variable-sized data members.

I do like the struct approach, because it allows the compiler to do more of the typechecking and offset calculation, and makes it easy to add other fields if needed, and should end up generating the same machine instructions as doing it 'by hand'. But it still requires an untyped allocation underneath (either malloc() or new char[]).

[Wim Lewis (wiml-omni)]

Ah, here's a simpler idea: simply store the 'drizzle_field_t' directly in the row structure, instead of a pointer to it; and use your typed allocator of choice for field_sizes.