Drizzle Client & Protocol Library

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

  1. 5.1-fix-null-bitmap
  2. Merge into libdrizzle-redux
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.
lp:~linuxjedi/libdrizzle/5.1-fix-null-bitmap updated
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
1=== modified file 'libdrizzle/field.cc'
2--- libdrizzle/field.cc 2013-03-12 16:27:58 +0000
3+++ libdrizzle/field.cc 2013-04-22 19:16:28 +0000
4@@ -66,7 +66,7 @@
5
6 if (result->has_state())
7 {
8- if (result->field_current == result->column_count)
9+ if (result->field_current == (result->column_count - result->null_bitcount))
10 {
11 *ret_ptr= DRIZZLE_RETURN_ROW_END;
12 return NULL;
13@@ -324,11 +324,23 @@
14
15 drizzle_return_t drizzle_state_binary_null_read(drizzle_st *con)
16 {
17+ uint16_t bit_count= 0;
18 con->result->null_bitmap_length= (con->result->column_count+7+2)/8;
19 con->result->null_bitmap= new uint8_t[con->result->null_bitmap_length];
20 con->buffer_ptr++;
21
22 memcpy(con->result->null_bitmap, con->buffer_ptr, con->result->null_bitmap_length);
23+
24+ for (uint16_t it= 0; it < con->result->null_bitmap_length; it++)
25+ {
26+ uint8_t data= con->result->null_bitmap[it];
27+ while (data)
28+ {
29+ data &= (data - 1);
30+ bit_count++;
31+ }
32+ }
33+ con->result->null_bitcount = bit_count;
34 con->buffer_ptr+= con->result->null_bitmap_length;
35 con->buffer_size-= con->result->null_bitmap_length+1;
36 con->packet_size-= con->result->null_bitmap_length+1;
37
38=== modified file 'libdrizzle/result.h'
39--- libdrizzle/result.h 2013-03-12 00:55:59 +0000
40+++ libdrizzle/result.h 2013-04-22 19:16:28 +0000
41@@ -81,6 +81,7 @@
42 uint8_t **null_bitmap_list;
43 uint8_t *null_bitmap;
44 uint16_t null_bitmap_length;
45+ uint16_t null_bitcount;
46 bool binary_rows;
47
48 drizzle_result_st() :
49@@ -115,6 +116,7 @@
50 null_bitmap_list(NULL),
51 null_bitmap(NULL),
52 null_bitmap_length(0),
53+ null_bitcount(0),
54 binary_rows(false)
55 {
56 info[0]= '\0';
57
58=== modified file 'libdrizzle/row.cc'
59--- libdrizzle/row.cc 2013-01-27 11:55:48 +0000
60+++ libdrizzle/row.cc 2013-04-22 19:16:28 +0000
61@@ -155,11 +155,17 @@
62 return;
63 }
64
65- for (x= 0; x < result->column_count; x++)
66+ for (x= 0; x < (result->column_count - result->null_bitcount); x++)
67 {
68 drizzle_field_free(row[x]);
69 }
70
71+ if (!(result->options & DRIZZLE_RESULT_BUFFER_ROW))
72+ {
73+ delete[] result->null_bitmap;
74+ result->null_bitmap= NULL;
75+ }
76+
77 delete[] row;
78 }
79
80
81=== modified file 'libdrizzle/statement.cc'
82--- libdrizzle/statement.cc 2013-04-21 21:09:53 +0000
83+++ libdrizzle/statement.cc 2013-04-22 19:16:28 +0000
84@@ -105,6 +105,7 @@
85 drizzle_return_t drizzle_stmt_execute(drizzle_stmt_st *stmt)
86 {
87 uint16_t current_param;
88+ uint16_t param_count = stmt->param_count;
89 drizzle_bind_st *param_ptr;
90 size_t param_lengths= 0;
91 size_t buffer_size= 0;
92@@ -121,6 +122,12 @@
93 drizzle_set_error(stmt->con, __func__, "parameter %d has not been bound", current_param);
94 return DRIZZLE_RETURN_STMT_ERROR;
95 }
96+ /* Don't count NULLs */
97+ if (stmt->query_params[current_param].type == DRIZZLE_COLUMN_TYPE_NULL)
98+ {
99+ param_count--;
100+ continue;
101+ }
102 /* parameter length + 8 byte header */
103 param_lengths+= stmt->query_params[current_param].length + 8;
104 }
105@@ -130,7 +137,7 @@
106 + 4 /* Reserved (always set to 1) */
107 + stmt->null_bitmap_length /* Null bitmap length */
108 + 1 /* New parameters bound flag */
109- + (stmt->param_count * 2) /* Parameter type data */
110+ + (param_count * 2) /* Parameter type data */
111 + param_lengths; /* Parameter data */
112
113 buffer = new (std::nothrow) unsigned char[buffer_size];
114@@ -149,7 +156,6 @@
115 drizzle_set_byte4(&buffer[5], 1);
116 buffer_pos+= 9;
117 /* Null bitmap */
118- memcpy(buffer_pos, stmt->null_bitmap, stmt->null_bitmap_length);
119 buffer_pos+= stmt->null_bitmap_length;
120 /* New parameters bound flag
121 * If set to 1 then we need to leave a gap between the parameters type data
122@@ -162,7 +168,7 @@
123 buffer_pos++;
124 /* Each param has a 2 byte data type header so the data pointer should be
125 * moved to this */
126- data_pos= buffer_pos + (stmt->param_count * 2);
127+ data_pos= buffer_pos + (param_count * 2);
128 }
129 else
130 {
131@@ -170,7 +176,7 @@
132 buffer_pos++;
133 data_pos= buffer_pos;
134 }
135-
136+ memset(stmt->null_bitmap, 0, stmt->null_bitmap_length);
137 /* Go through each param copying to buffer
138 * In an ideal world we would do this without the copy
139 * */
140@@ -184,13 +190,16 @@
141 if (stmt->new_bind)
142 {
143 uint16_t type= (uint16_t)param_ptr->type;
144- if (param_ptr->options.is_unsigned)
145+ if (type != DRIZZLE_COLUMN_TYPE_NULL)
146 {
147- /* Set the unsigned bit flag on the type data */
148- type |= 0x8000;
149+ if (param_ptr->options.is_unsigned)
150+ {
151+ /* Set the unsigned bit flag on the type data */
152+ type |= 0x8000;
153+ }
154+ drizzle_set_byte2(buffer_pos, type);
155+ buffer_pos+= 2;
156 }
157- drizzle_set_byte2(buffer_pos, type);
158- buffer_pos+= 2;
159 }
160
161 if (param_ptr->options.is_long_data)
162@@ -203,7 +212,7 @@
163 {
164 case DRIZZLE_COLUMN_TYPE_NULL:
165 /* Toggle the bit for this column in the bitmap */
166- *stmt->null_bitmap |= (current_param ^ 2);
167+ stmt->null_bitmap[current_param/8] |= (1 << (current_param % 8));
168 break;
169 case DRIZZLE_COLUMN_TYPE_TINY:
170 *data_pos= *(uint8_t*)param_ptr->data;
171@@ -271,6 +280,8 @@
172 break;
173 }
174 }
175+ /* Copy NULL bitmap */
176+ memcpy(&buffer[9], stmt->null_bitmap, stmt->null_bitmap_length);
177 if (stmt->execute_result)
178 {
179 drizzle_result_free(stmt->execute_result);
180@@ -379,7 +390,7 @@
181 drizzle_return_t drizzle_stmt_fetch(drizzle_stmt_st *stmt)
182 {
183 drizzle_return_t ret= DRIZZLE_RETURN_OK;
184- uint16_t column_counter= 0;
185+ uint16_t column_counter= 0, current_column= 0;
186 drizzle_row_t row;
187 drizzle_column_st *column;
188
189@@ -413,22 +424,25 @@
190 }
191 drizzle_column_seek(stmt->execute_result, 0);
192
193- while((column= drizzle_column_next(stmt->execute_result)))
194+ for (column_counter = 0; column_counter < stmt->execute_result->column_count; column_counter++)
195 {
196 drizzle_bind_st *param= &stmt->result_params[column_counter];
197- param->type= column->type;
198- /* if this row is null in the result bitmap */
199- if (*stmt->execute_result->null_bitmap & ((column_counter^2) << 2))
200+ /* if this row is null in the result bitmap, skip first 2 bits */
201+ if (stmt->execute_result->null_bitmap[(column_counter+2)/8] & (1 << ((column_counter+2) % 8)))
202 {
203 param->options.is_null= true;
204 param->length= 0;
205+ param->type= DRIZZLE_COLUMN_TYPE_NULL;
206 }
207 else
208 {
209 uint16_t short_data;
210 uint32_t long_data;
211 uint64_t longlong_data;
212- param->length= stmt->execute_result->field_sizes[column_counter];
213+ column= drizzle_column_next(stmt->execute_result);
214+ param->type= column->type;
215+ param->options.is_null= false;
216+ param->length= stmt->execute_result->field_sizes[current_column];
217 if (column->flags & DRIZZLE_COLUMN_FLAGS_UNSIGNED)
218 {
219 param->options.is_unsigned= true;
220@@ -442,42 +456,42 @@
221 break;
222 case DRIZZLE_COLUMN_TYPE_TINY:
223 param->data= param->data_buffer;
224- *(uint8_t*)param->data= *row[column_counter];
225+ *(uint8_t*)param->data= *row[current_column];
226 break;
227 case DRIZZLE_COLUMN_TYPE_SHORT:
228 case DRIZZLE_COLUMN_TYPE_YEAR:
229 param->data= param->data_buffer;
230- short_data= drizzle_get_byte2(row[column_counter]);
231+ short_data= drizzle_get_byte2(row[current_column]);
232 memcpy(param->data, &short_data, 2);
233 break;
234 case DRIZZLE_COLUMN_TYPE_INT24:
235 case DRIZZLE_COLUMN_TYPE_LONG:
236 param->data= param->data_buffer;
237- long_data= drizzle_get_byte4(row[column_counter]);
238+ long_data= drizzle_get_byte4(row[current_column]);
239 memcpy(param->data, &long_data, 4);
240 break;
241 case DRIZZLE_COLUMN_TYPE_LONGLONG:
242 param->data= param->data_buffer;
243- longlong_data= drizzle_get_byte8(row[column_counter]);
244+ longlong_data= drizzle_get_byte8(row[current_column]);
245 memcpy(param->data, &longlong_data, 8);
246 break;
247 case DRIZZLE_COLUMN_TYPE_FLOAT:
248 param->data= param->data_buffer;
249- memcpy(param->data, row[column_counter], 4);
250+ memcpy(param->data, row[current_column], 4);
251 break;
252 case DRIZZLE_COLUMN_TYPE_DOUBLE:
253 param->data= param->data_buffer;
254- memcpy(param->data, row[column_counter], 8);
255+ memcpy(param->data, row[current_column], 8);
256 break;
257 case DRIZZLE_COLUMN_TYPE_TIME:
258 param->data= param->data_buffer;
259- drizzle_unpack_time(row[column_counter], param->length, (drizzle_datetime_st *)param->data);
260+ drizzle_unpack_time(row[current_column], param->length, (drizzle_datetime_st *)param->data);
261 break;
262 case DRIZZLE_COLUMN_TYPE_DATE:
263 case DRIZZLE_COLUMN_TYPE_DATETIME:
264 case DRIZZLE_COLUMN_TYPE_TIMESTAMP:
265 param->data= param->data_buffer;
266- drizzle_unpack_datetime(row[column_counter], param->length, (drizzle_datetime_st *)param->data);
267+ drizzle_unpack_datetime(row[current_column], param->length, (drizzle_datetime_st *)param->data);
268 break;
269 case DRIZZLE_COLUMN_TYPE_TINY_BLOB:
270 case DRIZZLE_COLUMN_TYPE_MEDIUM_BLOB:
271@@ -489,7 +503,7 @@
272 case DRIZZLE_COLUMN_TYPE_DECIMAL:
273 case DRIZZLE_COLUMN_TYPE_NEWDECIMAL:
274 case DRIZZLE_COLUMN_TYPE_NEWDATE:
275- param->data= row[column_counter];
276+ param->data= row[current_column];
277 break;
278 /* These types aren't handled yet, most are for older MySQL versions */
279 case DRIZZLE_COLUMN_TYPE_VARCHAR:
280@@ -505,8 +519,8 @@
281 ret= DRIZZLE_RETURN_UNEXPECTED_DATA;
282 break;
283 }
284+ current_column++;
285 }
286- column_counter++;
287 if (ret == DRIZZLE_RETURN_UNEXPECTED_DATA)
288 {
289 break;
290
291=== modified file 'libdrizzle/statement_param.cc'
292--- libdrizzle/statement_param.cc 2013-04-21 21:09:31 +0000
293+++ libdrizzle/statement_param.cc 2013-04-22 19:16:28 +0000
294@@ -41,7 +41,7 @@
295 /* Internal function */
296 drizzle_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)
297 {
298- if ((stmt == NULL) || (param_num >= stmt->param_count) || (data == NULL))
299+ if ((stmt == NULL) || (param_num >= stmt->param_count))
300 {
301 return DRIZZLE_RETURN_INVALID_ARGUMENT;
302 }
303
304=== modified file 'tests/unit/include.am'
305--- tests/unit/include.am 2013-04-21 21:41:39 +0000
306+++ tests/unit/include.am 2013-04-22 19:16:28 +0000
307@@ -63,8 +63,6 @@
308 tests_unit_nulls_LDADD= libdrizzle/libdrizzle.la
309 check_PROGRAMS+= tests/unit/nulls
310 noinst_PROGRAMS+= tests/unit/nulls
311-# Currently fails due to https://bugs.launchpad.net/libdrizzle/+bug/1150195
312-XFAIL_TESTS+= tests/unit/nulls
313
314 tests_unit_row_SOURCES= tests/unit/row.c
315 tests_unit_row_LDADD= libdrizzle/libdrizzle.la
316
317=== modified file 'tests/unit/nulls.c'
318--- tests/unit/nulls.c 2013-03-06 03:06:26 +0000
319+++ tests/unit/nulls.c 2013-04-22 19:16:28 +0000
320@@ -96,7 +96,7 @@
321
322 #define NCOLS 11
323
324- char *querybuf = malloc(256 + 60*64);
325+ char *querybuf = calloc(256 + 60*64, 1);
326 strcpy(querybuf, "insert into libdrizzle.t1 values ");
327 char *p = querybuf + strlen(querybuf);
328
329@@ -241,7 +241,7 @@
330 uint32_t rowvalue;
331
332 #define GETNULLNESS(cn) isNull = drizzle_stmt_get_is_null(sth, cn, &driz_ret); ASSERT_EQ(driz_ret, DRIZZLE_RETURN_OK);
333-#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));
334+#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);
335 #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); }
336 #define NULLNEVER(cn) GETNULLNESS(cn); NULLNOTNOW(cn);
337
338@@ -262,7 +262,7 @@
339 #undef GETNULLNESS
340 #undef NULLNOTNOW
341 }
342- ASSERT_EQ(cur_row, table_size);
343+ ASSERT_EQ_(cur_row, table_size, "Current row not equal to table size, expected %d, got %d", table_size, cur_row);
344 ASSERT_EQ(drizzle_stmt_close(sth), DRIZZLE_RETURN_OK);
345 #endif
346
347@@ -273,6 +273,7 @@
348
349 driz_ret= drizzle_quit(con);
350 ASSERT_EQ_(DRIZZLE_RETURN_OK, driz_ret, "%s", drizzle_strerror(driz_ret));
351-
352+
353+ free(querybuf);
354 return EXIT_SUCCESS;
355 }

Subscribers

People subscribed via source and target branches

to all changes:
to status/vote changes: