Wrong-size allocation in row.cc
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
| Drizzle Client & Protocol Library |
Medium
|
Andrew Hutchings |
Bug Description
row.cc [ http://
result->row= new (std::nothrow) drizzle_
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(
struct row_info {
drizzle_field_t row;
size_t sizes[1];
}
make result->row be a pointer to this structure, and malloc(
Related branches
- Drizzle Trunk: Pending requested 2013-04-25
-
Diff: 144 lines (+19/-12)7 files modifiedlibdrizzle/datetime.h (+1/-0)
libdrizzle/pack.cc (+10/-2)
libdrizzle/pack.h (+2/-2)
libdrizzle/statement.cc (+2/-2)
libdrizzle/statement_param.cc (+3/-4)
tests/unit/datetypes.c (+1/-1)
tests/unit/include.am (+0/-1)
Andrew Hutchings (linuxjedi) wrote : | #1 |
Wim Lewis (wiml-omni) wrote : | #2 |
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) wrote : | #3 |
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.
Changed in libdrizzle: | |
importance: | Undecided → Medium |
assignee: | nobody → Andrew Hutchings (linuxjedi) |
milestone: | none → 5.1.4 |
status: | New → Triaged |
Changed in libdrizzle: | |
status: | Triaged → Fix Released |
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