Skip to content

Commit 6614ad3

Browse files
spetruniaGunnar Kudrjavets
authored andcommitted
Issue #15: SIGSEGV from reading in blob data
- convert_record_from_storage_format(buf) worked incorrectly when buf != table->record[0]. Length bytes were written to table->record[0] still. Moved field->move_field_offset() call earlier to prevent that. - Also, added comments and free() calls to free the retrieved blobs (the API is undocumented so we copy what InnoDB does).
1 parent 610d511 commit 6614ad3

File tree

2 files changed

+27
-3
lines changed

2 files changed

+27
-3
lines changed

storage/rocksdb/ha_rocksdb.cc

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,16 +1073,20 @@ int ha_rocksdb::convert_record_from_storage_format(uchar * buf)
10731073
const char *data_len_str;
10741074
if (!(data_len_str= reader.read(length_bytes)))
10751075
return HA_ERR_INTERNAL_ERROR;
1076+
1077+
blob->move_field_offset(ptr_diff);
10761078
memcpy(blob->ptr, data_len_str, length_bytes);
10771079

10781080
uint32 data_len= blob->get_length((uchar*)data_len_str, length_bytes,
10791081
table->s->db_low_byte_first);
10801082
const char *blob_ptr;
10811083
if (!(blob_ptr= reader.read(data_len)))
1084+
{
1085+
blob->move_field_offset(-ptr_diff);
10821086
return HA_ERR_INTERNAL_ERROR;
1087+
}
10831088

1084-
blob->move_field_offset(ptr_diff);
1085-
// set 8-byte pointer to 0, like innodb does.
1089+
// set 8-byte pointer to 0, like innodb does (relevant for 32-bit platforms)
10861090
memset(blob->ptr + length_bytes, 0, 8);
10871091
memcpy(blob->ptr + length_bytes, &blob_ptr, sizeof(uchar**));
10881092
blob->move_field_offset(-ptr_diff);
@@ -2836,6 +2840,13 @@ int ha_rocksdb::extra(enum ha_extra_function operation)
28362840
case HA_EXTRA_NO_KEYREAD:
28372841
keyread_only= FALSE;
28382842
break;
2843+
case HA_EXTRA_FLUSH:
2844+
/*
2845+
If the table has blobs, then they are part of retrieved_record. This call
2846+
invalidates them.
2847+
*/
2848+
retrieved_record.clear();
2849+
break;
28392850
default:
28402851
break;
28412852
}

storage/rocksdb/ha_rocksdb.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,13 @@ class ha_rocksdb: public handler
122122
/* Buffer used by convert_record_to_storage_format() */
123123
String storage_record;
124124

125-
/* Last retrieved record, in table->record[0] data format */
125+
/*
126+
Last retrieved record, in table->record[0] data format. It also serves as
127+
as storage for blob data (Field_blob object have pointers to here)
128+
129+
TODO: Dont we lose one malloc() per record read by having it as std::string
130+
instead of rocksdb::Slice?
131+
*/
126132
std::string retrieved_record;
127133

128134
/* If TRUE, reads should place locks on rows */
@@ -312,6 +318,13 @@ class ha_rocksdb: public handler
312318
int delete_all_rows(void);
313319
int truncate();
314320

321+
int reset()
322+
{
323+
/* Free blob data */
324+
retrieved_record.clear();
325+
return 0;
326+
}
327+
315328
void remove_rows(RDBSE_TABLE_DEF *tbl);
316329
ha_rows records_in_range(uint inx, key_range *min_key,
317330
key_range *max_key);

0 commit comments

Comments
 (0)