Merge lp:~linuxjedi/libdrizzle/5.1-fix-null-bitmap into lp:libdrizzle

Proposed by Andrew Hutchings
Status: Merged
Approved by: Andrew Hutchings
Approved revision: 112
Merged at revision: 112
Proposed branch: lp:~linuxjedi/libdrizzle/5.1-fix-null-bitmap
Merge into: lp:libdrizzle
Diff against target: 355 lines (+68/-35)
7 files modified
libdrizzle/field.cc (+13/-1)
libdrizzle/result.h (+2/-0)
libdrizzle/row.cc (+7/-1)
libdrizzle/statement.cc (+40/-26)
libdrizzle/statement_param.cc (+1/-1)
tests/unit/include.am (+0/-2)
tests/unit/nulls.c (+5/-4)
To merge this branch: bzr merge lp:~linuxjedi/libdrizzle/5.1-fix-null-bitmap
Reviewer Review Type Date Requested Status
Drizzle Trunk Pending
Review via email: mp+160176@code.launchpad.net

Description of the change

Fixes many bugs that break NULL data in prepared statements.

To post a comment you must log in.
113. By Andrew Hutchings

Fix valgrind errors in prep statement NULL handling

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'libdrizzle/field.cc'
--- libdrizzle/field.cc 2013-03-12 16:27:58 +0000
+++ libdrizzle/field.cc 2013-04-22 19:16:28 +0000
@@ -66,7 +66,7 @@
6666
67 if (result->has_state())67 if (result->has_state())
68 {68 {
69 if (result->field_current == result->column_count)69 if (result->field_current == (result->column_count - result->null_bitcount))
70 {70 {
71 *ret_ptr= DRIZZLE_RETURN_ROW_END;71 *ret_ptr= DRIZZLE_RETURN_ROW_END;
72 return NULL;72 return NULL;
@@ -324,11 +324,23 @@
324324
325drizzle_return_t drizzle_state_binary_null_read(drizzle_st *con)325drizzle_return_t drizzle_state_binary_null_read(drizzle_st *con)
326{326{
327 uint16_t bit_count= 0;
327 con->result->null_bitmap_length= (con->result->column_count+7+2)/8;328 con->result->null_bitmap_length= (con->result->column_count+7+2)/8;
328 con->result->null_bitmap= new uint8_t[con->result->null_bitmap_length];329 con->result->null_bitmap= new uint8_t[con->result->null_bitmap_length];
329 con->buffer_ptr++;330 con->buffer_ptr++;
330331
331 memcpy(con->result->null_bitmap, con->buffer_ptr, con->result->null_bitmap_length);332 memcpy(con->result->null_bitmap, con->buffer_ptr, con->result->null_bitmap_length);
333
334 for (uint16_t it= 0; it < con->result->null_bitmap_length; it++)
335 {
336 uint8_t data= con->result->null_bitmap[it];
337 while (data)
338 {
339 data &= (data - 1);
340 bit_count++;
341 }
342 }
343 con->result->null_bitcount = bit_count;
332 con->buffer_ptr+= con->result->null_bitmap_length;344 con->buffer_ptr+= con->result->null_bitmap_length;
333 con->buffer_size-= con->result->null_bitmap_length+1;345 con->buffer_size-= con->result->null_bitmap_length+1;
334 con->packet_size-= con->result->null_bitmap_length+1;346 con->packet_size-= con->result->null_bitmap_length+1;
335347
=== modified file 'libdrizzle/result.h'
--- libdrizzle/result.h 2013-03-12 00:55:59 +0000
+++ libdrizzle/result.h 2013-04-22 19:16:28 +0000
@@ -81,6 +81,7 @@
81 uint8_t **null_bitmap_list;81 uint8_t **null_bitmap_list;
82 uint8_t *null_bitmap;82 uint8_t *null_bitmap;
83 uint16_t null_bitmap_length;83 uint16_t null_bitmap_length;
84 uint16_t null_bitcount;
84 bool binary_rows;85 bool binary_rows;
8586
86 drizzle_result_st() :87 drizzle_result_st() :
@@ -115,6 +116,7 @@
115 null_bitmap_list(NULL),116 null_bitmap_list(NULL),
116 null_bitmap(NULL),117 null_bitmap(NULL),
117 null_bitmap_length(0),118 null_bitmap_length(0),
119 null_bitcount(0),
118 binary_rows(false)120 binary_rows(false)
119 {121 {
120 info[0]= '\0';122 info[0]= '\0';
121123
=== modified file 'libdrizzle/row.cc'
--- libdrizzle/row.cc 2013-01-27 11:55:48 +0000
+++ libdrizzle/row.cc 2013-04-22 19:16:28 +0000
@@ -155,11 +155,17 @@
155 return;155 return;
156 }156 }
157157
158 for (x= 0; x < result->column_count; x++)158 for (x= 0; x < (result->column_count - result->null_bitcount); x++)
159 {159 {
160 drizzle_field_free(row[x]);160 drizzle_field_free(row[x]);
161 }161 }
162162
163 if (!(result->options & DRIZZLE_RESULT_BUFFER_ROW))
164 {
165 delete[] result->null_bitmap;
166 result->null_bitmap= NULL;
167 }
168
163 delete[] row;169 delete[] row;
164}170}
165171
166172
=== modified file 'libdrizzle/statement.cc'
--- libdrizzle/statement.cc 2013-04-21 21:09:53 +0000
+++ libdrizzle/statement.cc 2013-04-22 19:16:28 +0000
@@ -105,6 +105,7 @@
105drizzle_return_t drizzle_stmt_execute(drizzle_stmt_st *stmt)105drizzle_return_t drizzle_stmt_execute(drizzle_stmt_st *stmt)
106{106{
107 uint16_t current_param;107 uint16_t current_param;
108 uint16_t param_count = stmt->param_count;
108 drizzle_bind_st *param_ptr;109 drizzle_bind_st *param_ptr;
109 size_t param_lengths= 0;110 size_t param_lengths= 0;
110 size_t buffer_size= 0;111 size_t buffer_size= 0;
@@ -121,6 +122,12 @@
121 drizzle_set_error(stmt->con, __func__, "parameter %d has not been bound", current_param);122 drizzle_set_error(stmt->con, __func__, "parameter %d has not been bound", current_param);
122 return DRIZZLE_RETURN_STMT_ERROR;123 return DRIZZLE_RETURN_STMT_ERROR;
123 }124 }
125 /* Don't count NULLs */
126 if (stmt->query_params[current_param].type == DRIZZLE_COLUMN_TYPE_NULL)
127 {
128 param_count--;
129 continue;
130 }
124 /* parameter length + 8 byte header */131 /* parameter length + 8 byte header */
125 param_lengths+= stmt->query_params[current_param].length + 8;132 param_lengths+= stmt->query_params[current_param].length + 8;
126 }133 }
@@ -130,7 +137,7 @@
130 + 4 /* Reserved (always set to 1) */137 + 4 /* Reserved (always set to 1) */
131 + stmt->null_bitmap_length /* Null bitmap length */138 + stmt->null_bitmap_length /* Null bitmap length */
132 + 1 /* New parameters bound flag */139 + 1 /* New parameters bound flag */
133 + (stmt->param_count * 2) /* Parameter type data */140 + (param_count * 2) /* Parameter type data */
134 + param_lengths; /* Parameter data */141 + param_lengths; /* Parameter data */
135142
136 buffer = new (std::nothrow) unsigned char[buffer_size];143 buffer = new (std::nothrow) unsigned char[buffer_size];
@@ -149,7 +156,6 @@
149 drizzle_set_byte4(&buffer[5], 1);156 drizzle_set_byte4(&buffer[5], 1);
150 buffer_pos+= 9;157 buffer_pos+= 9;
151 /* Null bitmap */158 /* Null bitmap */
152 memcpy(buffer_pos, stmt->null_bitmap, stmt->null_bitmap_length);
153 buffer_pos+= stmt->null_bitmap_length;159 buffer_pos+= stmt->null_bitmap_length;
154 /* New parameters bound flag160 /* New parameters bound flag
155 * If set to 1 then we need to leave a gap between the parameters type data161 * If set to 1 then we need to leave a gap between the parameters type data
@@ -162,7 +168,7 @@
162 buffer_pos++;168 buffer_pos++;
163 /* Each param has a 2 byte data type header so the data pointer should be169 /* Each param has a 2 byte data type header so the data pointer should be
164 * moved to this */170 * moved to this */
165 data_pos= buffer_pos + (stmt->param_count * 2);171 data_pos= buffer_pos + (param_count * 2);
166 }172 }
167 else173 else
168 {174 {
@@ -170,7 +176,7 @@
170 buffer_pos++;176 buffer_pos++;
171 data_pos= buffer_pos;177 data_pos= buffer_pos;
172 }178 }
173 179 memset(stmt->null_bitmap, 0, stmt->null_bitmap_length);
174 /* Go through each param copying to buffer 180 /* Go through each param copying to buffer
175 * In an ideal world we would do this without the copy181 * In an ideal world we would do this without the copy
176 * */182 * */
@@ -184,13 +190,16 @@
184 if (stmt->new_bind)190 if (stmt->new_bind)
185 {191 {
186 uint16_t type= (uint16_t)param_ptr->type;192 uint16_t type= (uint16_t)param_ptr->type;
187 if (param_ptr->options.is_unsigned)193 if (type != DRIZZLE_COLUMN_TYPE_NULL)
188 {194 {
189 /* Set the unsigned bit flag on the type data */195 if (param_ptr->options.is_unsigned)
190 type |= 0x8000;196 {
197 /* Set the unsigned bit flag on the type data */
198 type |= 0x8000;
199 }
200 drizzle_set_byte2(buffer_pos, type);
201 buffer_pos+= 2;
191 }202 }
192 drizzle_set_byte2(buffer_pos, type);
193 buffer_pos+= 2;
194 }203 }
195204
196 if (param_ptr->options.is_long_data)205 if (param_ptr->options.is_long_data)
@@ -203,7 +212,7 @@
203 {212 {
204 case DRIZZLE_COLUMN_TYPE_NULL:213 case DRIZZLE_COLUMN_TYPE_NULL:
205 /* Toggle the bit for this column in the bitmap */214 /* Toggle the bit for this column in the bitmap */
206 *stmt->null_bitmap |= (current_param ^ 2);215 stmt->null_bitmap[current_param/8] |= (1 << (current_param % 8));
207 break;216 break;
208 case DRIZZLE_COLUMN_TYPE_TINY:217 case DRIZZLE_COLUMN_TYPE_TINY:
209 *data_pos= *(uint8_t*)param_ptr->data;218 *data_pos= *(uint8_t*)param_ptr->data;
@@ -271,6 +280,8 @@
271 break;280 break;
272 }281 }
273 }282 }
283 /* Copy NULL bitmap */
284 memcpy(&buffer[9], stmt->null_bitmap, stmt->null_bitmap_length);
274 if (stmt->execute_result)285 if (stmt->execute_result)
275 {286 {
276 drizzle_result_free(stmt->execute_result);287 drizzle_result_free(stmt->execute_result);
@@ -379,7 +390,7 @@
379drizzle_return_t drizzle_stmt_fetch(drizzle_stmt_st *stmt)390drizzle_return_t drizzle_stmt_fetch(drizzle_stmt_st *stmt)
380{391{
381 drizzle_return_t ret= DRIZZLE_RETURN_OK;392 drizzle_return_t ret= DRIZZLE_RETURN_OK;
382 uint16_t column_counter= 0;393 uint16_t column_counter= 0, current_column= 0;
383 drizzle_row_t row;394 drizzle_row_t row;
384 drizzle_column_st *column;395 drizzle_column_st *column;
385396
@@ -413,22 +424,25 @@
413 }424 }
414 drizzle_column_seek(stmt->execute_result, 0);425 drizzle_column_seek(stmt->execute_result, 0);
415426
416 while((column= drizzle_column_next(stmt->execute_result)))427 for (column_counter = 0; column_counter < stmt->execute_result->column_count; column_counter++)
417 {428 {
418 drizzle_bind_st *param= &stmt->result_params[column_counter];429 drizzle_bind_st *param= &stmt->result_params[column_counter];
419 param->type= column->type;430 /* if this row is null in the result bitmap, skip first 2 bits */
420 /* if this row is null in the result bitmap */431 if (stmt->execute_result->null_bitmap[(column_counter+2)/8] & (1 << ((column_counter+2) % 8)))
421 if (*stmt->execute_result->null_bitmap & ((column_counter^2) << 2))
422 {432 {
423 param->options.is_null= true;433 param->options.is_null= true;
424 param->length= 0;434 param->length= 0;
435 param->type= DRIZZLE_COLUMN_TYPE_NULL;
425 }436 }
426 else437 else
427 {438 {
428 uint16_t short_data;439 uint16_t short_data;
429 uint32_t long_data;440 uint32_t long_data;
430 uint64_t longlong_data;441 uint64_t longlong_data;
431 param->length= stmt->execute_result->field_sizes[column_counter];442 column= drizzle_column_next(stmt->execute_result);
443 param->type= column->type;
444 param->options.is_null= false;
445 param->length= stmt->execute_result->field_sizes[current_column];
432 if (column->flags & DRIZZLE_COLUMN_FLAGS_UNSIGNED)446 if (column->flags & DRIZZLE_COLUMN_FLAGS_UNSIGNED)
433 {447 {
434 param->options.is_unsigned= true;448 param->options.is_unsigned= true;
@@ -442,42 +456,42 @@
442 break;456 break;
443 case DRIZZLE_COLUMN_TYPE_TINY:457 case DRIZZLE_COLUMN_TYPE_TINY:
444 param->data= param->data_buffer;458 param->data= param->data_buffer;
445 *(uint8_t*)param->data= *row[column_counter];459 *(uint8_t*)param->data= *row[current_column];
446 break;460 break;
447 case DRIZZLE_COLUMN_TYPE_SHORT:461 case DRIZZLE_COLUMN_TYPE_SHORT:
448 case DRIZZLE_COLUMN_TYPE_YEAR:462 case DRIZZLE_COLUMN_TYPE_YEAR:
449 param->data= param->data_buffer;463 param->data= param->data_buffer;
450 short_data= drizzle_get_byte2(row[column_counter]);464 short_data= drizzle_get_byte2(row[current_column]);
451 memcpy(param->data, &short_data, 2);465 memcpy(param->data, &short_data, 2);
452 break;466 break;
453 case DRIZZLE_COLUMN_TYPE_INT24:467 case DRIZZLE_COLUMN_TYPE_INT24:
454 case DRIZZLE_COLUMN_TYPE_LONG:468 case DRIZZLE_COLUMN_TYPE_LONG:
455 param->data= param->data_buffer;469 param->data= param->data_buffer;
456 long_data= drizzle_get_byte4(row[column_counter]);470 long_data= drizzle_get_byte4(row[current_column]);
457 memcpy(param->data, &long_data, 4);471 memcpy(param->data, &long_data, 4);
458 break;472 break;
459 case DRIZZLE_COLUMN_TYPE_LONGLONG:473 case DRIZZLE_COLUMN_TYPE_LONGLONG:
460 param->data= param->data_buffer;474 param->data= param->data_buffer;
461 longlong_data= drizzle_get_byte8(row[column_counter]);475 longlong_data= drizzle_get_byte8(row[current_column]);
462 memcpy(param->data, &longlong_data, 8);476 memcpy(param->data, &longlong_data, 8);
463 break;477 break;
464 case DRIZZLE_COLUMN_TYPE_FLOAT:478 case DRIZZLE_COLUMN_TYPE_FLOAT:
465 param->data= param->data_buffer;479 param->data= param->data_buffer;
466 memcpy(param->data, row[column_counter], 4);480 memcpy(param->data, row[current_column], 4);
467 break;481 break;
468 case DRIZZLE_COLUMN_TYPE_DOUBLE:482 case DRIZZLE_COLUMN_TYPE_DOUBLE:
469 param->data= param->data_buffer;483 param->data= param->data_buffer;
470 memcpy(param->data, row[column_counter], 8);484 memcpy(param->data, row[current_column], 8);
471 break;485 break;
472 case DRIZZLE_COLUMN_TYPE_TIME:486 case DRIZZLE_COLUMN_TYPE_TIME:
473 param->data= param->data_buffer;487 param->data= param->data_buffer;
474 drizzle_unpack_time(row[column_counter], param->length, (drizzle_datetime_st *)param->data);488 drizzle_unpack_time(row[current_column], param->length, (drizzle_datetime_st *)param->data);
475 break;489 break;
476 case DRIZZLE_COLUMN_TYPE_DATE:490 case DRIZZLE_COLUMN_TYPE_DATE:
477 case DRIZZLE_COLUMN_TYPE_DATETIME:491 case DRIZZLE_COLUMN_TYPE_DATETIME:
478 case DRIZZLE_COLUMN_TYPE_TIMESTAMP:492 case DRIZZLE_COLUMN_TYPE_TIMESTAMP:
479 param->data= param->data_buffer;493 param->data= param->data_buffer;
480 drizzle_unpack_datetime(row[column_counter], param->length, (drizzle_datetime_st *)param->data);494 drizzle_unpack_datetime(row[current_column], param->length, (drizzle_datetime_st *)param->data);
481 break;495 break;
482 case DRIZZLE_COLUMN_TYPE_TINY_BLOB:496 case DRIZZLE_COLUMN_TYPE_TINY_BLOB:
483 case DRIZZLE_COLUMN_TYPE_MEDIUM_BLOB:497 case DRIZZLE_COLUMN_TYPE_MEDIUM_BLOB:
@@ -489,7 +503,7 @@
489 case DRIZZLE_COLUMN_TYPE_DECIMAL:503 case DRIZZLE_COLUMN_TYPE_DECIMAL:
490 case DRIZZLE_COLUMN_TYPE_NEWDECIMAL:504 case DRIZZLE_COLUMN_TYPE_NEWDECIMAL:
491 case DRIZZLE_COLUMN_TYPE_NEWDATE:505 case DRIZZLE_COLUMN_TYPE_NEWDATE:
492 param->data= row[column_counter];506 param->data= row[current_column];
493 break;507 break;
494 /* These types aren't handled yet, most are for older MySQL versions */508 /* These types aren't handled yet, most are for older MySQL versions */
495 case DRIZZLE_COLUMN_TYPE_VARCHAR:509 case DRIZZLE_COLUMN_TYPE_VARCHAR:
@@ -505,8 +519,8 @@
505 ret= DRIZZLE_RETURN_UNEXPECTED_DATA;519 ret= DRIZZLE_RETURN_UNEXPECTED_DATA;
506 break;520 break;
507 }521 }
522 current_column++;
508 }523 }
509 column_counter++;
510 if (ret == DRIZZLE_RETURN_UNEXPECTED_DATA)524 if (ret == DRIZZLE_RETURN_UNEXPECTED_DATA)
511 {525 {
512 break;526 break;
513527
=== modified file 'libdrizzle/statement_param.cc'
--- libdrizzle/statement_param.cc 2013-04-21 21:09:31 +0000
+++ libdrizzle/statement_param.cc 2013-04-22 19:16:28 +0000
@@ -41,7 +41,7 @@
41/* Internal function */41/* Internal function */
42drizzle_return_t drizzle_stmt_set_param(drizzle_stmt_st *stmt, uint16_t param_num, drizzle_column_type_t type, void *data, uint32_t length, bool is_unsigned)42drizzle_return_t drizzle_stmt_set_param(drizzle_stmt_st *stmt, uint16_t param_num, drizzle_column_type_t type, void *data, uint32_t length, bool is_unsigned)
43{43{
44 if ((stmt == NULL) || (param_num >= stmt->param_count) || (data == NULL))44 if ((stmt == NULL) || (param_num >= stmt->param_count))
45 {45 {
46 return DRIZZLE_RETURN_INVALID_ARGUMENT;46 return DRIZZLE_RETURN_INVALID_ARGUMENT;
47 }47 }
4848
=== modified file 'tests/unit/include.am'
--- tests/unit/include.am 2013-04-21 21:41:39 +0000
+++ tests/unit/include.am 2013-04-22 19:16:28 +0000
@@ -63,8 +63,6 @@
63tests_unit_nulls_LDADD= libdrizzle/libdrizzle.la63tests_unit_nulls_LDADD= libdrizzle/libdrizzle.la
64check_PROGRAMS+= tests/unit/nulls64check_PROGRAMS+= tests/unit/nulls
65noinst_PROGRAMS+= tests/unit/nulls65noinst_PROGRAMS+= tests/unit/nulls
66# Currently fails due to https://bugs.launchpad.net/libdrizzle/+bug/1150195
67XFAIL_TESTS+= tests/unit/nulls
6866
69tests_unit_row_SOURCES= tests/unit/row.c67tests_unit_row_SOURCES= tests/unit/row.c
70tests_unit_row_LDADD= libdrizzle/libdrizzle.la68tests_unit_row_LDADD= libdrizzle/libdrizzle.la
7169
=== modified file 'tests/unit/nulls.c'
--- tests/unit/nulls.c 2013-03-06 03:06:26 +0000
+++ tests/unit/nulls.c 2013-04-22 19:16:28 +0000
@@ -96,7 +96,7 @@
96 96
97#define NCOLS 1197#define NCOLS 11
98 98
99 char *querybuf = malloc(256 + 60*64);99 char *querybuf = calloc(256 + 60*64, 1);
100 strcpy(querybuf, "insert into libdrizzle.t1 values ");100 strcpy(querybuf, "insert into libdrizzle.t1 values ");
101 char *p = querybuf + strlen(querybuf);101 char *p = querybuf + strlen(querybuf);
102 102
@@ -241,7 +241,7 @@
241 uint32_t rowvalue;241 uint32_t rowvalue;
242 242
243#define GETNULLNESS(cn) isNull = drizzle_stmt_get_is_null(sth, cn, &driz_ret); ASSERT_EQ(driz_ret, DRIZZLE_RETURN_OK);243#define GETNULLNESS(cn) isNull = drizzle_stmt_get_is_null(sth, cn, &driz_ret); ASSERT_EQ(driz_ret, DRIZZLE_RETURN_OK);
244#define NULLNOTNOW(cn) ASSERT_FALSE_(isNull, "Column %d, row %d should not be NULL", cn+1, cur_row); rowvalue = drizzle_stmt_get_int(sth, cn, &driz_ret); ASSERT_EQ(driz_ret,DRIZZLE_RETURN_OK); ASSERT_EQ(rowvalue, (unsigned)(rowbase + cn));244#define NULLNOTNOW(cn) ASSERT_FALSE_(isNull, "Column %d, row %d should not be NULL", cn+1, cur_row); rowvalue = drizzle_stmt_get_int(sth, cn, &driz_ret); ASSERT_EQ(driz_ret,DRIZZLE_RETURN_OK); ASSERT_EQ_(rowvalue, (unsigned)(rowbase + cn), "Column %d, row %d has unexpected data, expected: %d, got: %d", cn+1, cur_row, rowbase+cn, rowvalue);
245#define NULLMAYBE(cn, b) GETNULLNESS(cn); if (sym & b) { ASSERT_TRUE_(isNull, "Column %d, row %d should be NULL", cn+1, cur_row); } else { NULLNOTNOW(cn); }245#define NULLMAYBE(cn, b) GETNULLNESS(cn); if (sym & b) { ASSERT_TRUE_(isNull, "Column %d, row %d should be NULL", cn+1, cur_row); } else { NULLNOTNOW(cn); }
246#define NULLNEVER(cn) GETNULLNESS(cn); NULLNOTNOW(cn);246#define NULLNEVER(cn) GETNULLNESS(cn); NULLNOTNOW(cn);
247 247
@@ -262,7 +262,7 @@
262#undef GETNULLNESS262#undef GETNULLNESS
263#undef NULLNOTNOW263#undef NULLNOTNOW
264 }264 }
265 ASSERT_EQ(cur_row, table_size);265 ASSERT_EQ_(cur_row, table_size, "Current row not equal to table size, expected %d, got %d", table_size, cur_row);
266 ASSERT_EQ(drizzle_stmt_close(sth), DRIZZLE_RETURN_OK);266 ASSERT_EQ(drizzle_stmt_close(sth), DRIZZLE_RETURN_OK);
267#endif267#endif
268 268
@@ -273,6 +273,7 @@
273273
274 driz_ret= drizzle_quit(con);274 driz_ret= drizzle_quit(con);
275 ASSERT_EQ_(DRIZZLE_RETURN_OK, driz_ret, "%s", drizzle_strerror(driz_ret));275 ASSERT_EQ_(DRIZZLE_RETURN_OK, driz_ret, "%s", drizzle_strerror(driz_ret));
276 276
277 free(querybuf);
277 return EXIT_SUCCESS;278 return EXIT_SUCCESS;
278}279}

Subscribers

People subscribed via source and target branches

to all changes:
to status/vote changes: