Skip to content

Commit

Permalink
replace stack struct members with allocations. This was causing memor…
Browse files Browse the repository at this point in the history 
…y corruption
  • Loading branch information
Aakash Arayambeth committed Feb 22, 2025
1 parent 2d848bb commit 3bce474
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 38 deletions.
9 changes: 6 additions & 3 deletions db/hash_partition.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ static void free_hash_view(hash_view_t *mView)
}

hash_view_t *create_hash_view(const char *viewname, const char *tablename, uint32_t num_columns,
char columns[][MAXCOLNAME], uint32_t num_partitions,
char partitions[][MAXPARTITIONLEN], struct errstat *err)
char **columns, uint32_t num_partitions,
char **partitions, struct errstat *err)
{
hash_view_t *mView;

Expand All @@ -94,6 +94,7 @@ hash_view_t *create_hash_view(const char *viewname, const char *tablename, uint3
goto oom;
}

logmsg(LOGMSG_USER, "CREATING A VIEW WITH NAME %s\n", viewname);
mView->viewname = strdup(viewname);
if (!mView->viewname) {
logmsg(LOGMSG_ERROR, "%s: Failed to allocate view name string %s\n", __func__, viewname);
Expand All @@ -113,6 +114,7 @@ hash_view_t *create_hash_view(const char *viewname, const char *tablename, uint3
}

for (int i = 0; i < mView->num_keys; i++) {
logmsg(LOGMSG_USER, "ADDING COLUMN %s\n", columns[i]);
mView->keynames[i] = strdup(columns[i]);
if (!mView->keynames[i]) {
logmsg(LOGMSG_ERROR, "%s: Failed to allocate key name string %s\n", __func__, columns[i]);
Expand All @@ -123,6 +125,7 @@ hash_view_t *create_hash_view(const char *viewname, const char *tablename, uint3

mView->partitions = (char **)calloc(1, sizeof(char *) * num_partitions);
for (int i = 0; i < num_partitions; i++) {
logmsg(LOGMSG_USER, "ADDING PARTITION %s\n", partitions[i]);
mView->partitions[i] = strdup(partitions[i]);
if (!mView->partitions[i]) {
goto oom;
Expand Down Expand Up @@ -323,7 +326,7 @@ int hash_partition_llmeta_write(void *tran, hash_view_t *view, struct errstat *e
goto done;
}

logmsg(LOGMSG_USER, "%s\n", view_str);
logmsg(LOGMSG_USER, "WRITING THE VIEWS STRING : %s TO LLMETA\n", view_str);
/* save the view */
rc = hash_views_write_view(tran, view->viewname, view_str, 0);
if (rc != VIEW_NOERR) {
Expand Down
4 changes: 2 additions & 2 deletions db/hash_partition.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ typedef struct systable_hashpartitions {
} systable_hashpartition_t;

hash_view_t *create_hash_view(const char *viewname, const char *tablename, uint32_t num_columns,
char columns[][MAXCOLNAME], uint32_t num_partitions,
char partitions[][MAXPARTITIONLEN], struct errstat *err);
char **columns, uint32_t num_partitions,
char **partitions, struct errstat *err);
int hash_create_inmem_view(hash_view_t *);
int hash_destroy_inmem_view(hash_view_t *);
const char *hash_view_get_viewname(struct hash_view *view);
Expand Down
9 changes: 9 additions & 0 deletions db/osqlcomm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -838,11 +838,19 @@ static uint8_t *osqlcomm_schemachange_rpl_type_put(osql_rpl_t *hd, struct schema
static uint8_t *osqlcomm_packed_schemachange_uuid_rpl_type_put(osql_uuid_rpl_t *hd, void *packed_sc, size_t sc_len,
uint8_t *p_buf, uint8_t *p_buf_end)
{

logmsg(LOGMSG_USER, "sc_len: %ld, p_buf_end - p_buf : %ld\n", sc_len, p_buf_end - p_buf);
if (p_buf_end < p_buf || OSQLCOMM_UUID_RPL_TYPE_LEN + sc_len > (p_buf_end - p_buf))
return NULL;

p_buf = osqlcomm_uuid_rpl_type_put(hd, p_buf, p_buf_end);
if (!p_buf) {
logmsg(LOGMSG_USER, "osqlcomm_uuid_rpl_type_put returned NULL\n");
}
p_buf = buf_no_net_put(packed_sc, sc_len, p_buf, p_buf_end);
if (!p_buf) {
logmsg(LOGMSG_USER, "buf_no_net_put returned NULL\n");
}

return p_buf;
}
Expand All @@ -852,6 +860,7 @@ static uint8_t *osqlcomm_schemachange_uuid_rpl_type_put(osql_uuid_rpl_t *hd, str
{
size_t sc_len = schemachange_packed_size(sc);

logmsg(LOGMSG_USER, "sc_len: %ld, p_buf_end - p_buf : %ld\n", sc_len, p_buf_end - p_buf);
if (p_buf_end < p_buf || OSQLCOMM_UUID_RPL_TYPE_LEN + sc_len > (p_buf_end - p_buf))
return NULL;

Expand Down
9 changes: 5 additions & 4 deletions db/views_serial.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1989,6 +1989,7 @@ hash_view_t *hash_deserialize_view(const char *view_str, struct errstat *err)
const char *err_str;
int num_partitions = 0, num_keys = 0;
int rc;
char **keynames = NULL, **dbnames = NULL;

/* parse string */
rc = cson_parse_string(&rootVal, view_str, strlen(view_str));
Expand Down Expand Up @@ -2028,14 +2029,14 @@ hash_view_t *hash_deserialize_view(const char *view_str, struct errstat *err)
goto error;
}

char keynames[MAXCOLUMNS][MAXCOLNAME];
keynames = (char **)malloc(sizeof(char*) * num_keys);
for (int i = 0; i < num_keys; i++) {
arrVal = cson_array_get(keys_arr, i);
if (!cson_value_is_string(arrVal)) {
err_str = "INVALID CSON. Array element is not a string";
goto error;
}
strcpy(keynames[i], cson_value_get_cstr(arrVal));
keynames[i] = strdup(cson_value_get_cstr(arrVal));
}

/* NUMPARTITIONS */
Expand All @@ -2052,15 +2053,15 @@ hash_view_t *hash_deserialize_view(const char *view_str, struct errstat *err)
goto error;
}

char dbnames[MAXPARTITIONS][MAXPARTITIONLEN];

dbnames = (char **)malloc(sizeof(char*) * num_partitions);
for (int i = 0; i < num_partitions; i++) {
arrVal = cson_array_get(partitions, i);
if (!cson_value_is_string(arrVal)) {
err_str = "INVALID CSON. Array element is not a string";
goto error;
}
strcpy(dbnames[i], cson_value_get_cstr(arrVal));
dbnames[i] = strdup(cson_value_get_cstr(arrVal));
}

view = create_hash_view(viewname, tablename, num_keys, keynames, num_partitions, dbnames, err);
Expand Down
53 changes: 39 additions & 14 deletions schemachange/sc_struct.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,15 @@ static size_t _partition_packed_size(struct comdb2_partition *p)
sizeof(p->u.mergetable.version);
case PARTITION_ADD_COL_HASH:
for (int i = 0; i < p->u.hash.num_partitions; i++) {
shardNamesSize += sizeof(p->u.hash.partitions[i]);
logmsg(LOGMSG_USER, "length of %s is %ld\n", p->u.hash.partitions[i], strlen(p->u.hash.partitions[i]));
shardNamesSize += sizeof(size_t) + strlen(p->u.hash.partitions[i]);
}

for (int i = 0; i < p->u.hash.num_columns; i++) {
columnNamesSize += sizeof(p->u.hash.columns[i]);
logmsg(LOGMSG_USER, "length of %s is %ld\n", p->u.hash.columns[i], strlen(p->u.hash.columns[i]));
columnNamesSize += sizeof(size_t) + strlen(p->u.hash.columns[i]);
}
return sizeof(p->type) + sizeof(p->u.hash.viewname) + sizeof(p->u.hash.num_partitions) +
return sizeof(p->type) + sizeof(size_t) + strlen(p->u.hash.viewname) + sizeof(p->u.hash.num_partitions) +
sizeof(p->u.hash.num_columns) + shardNamesSize + columnNamesSize;
default:
logmsg(LOGMSG_ERROR, "Unimplemented partition type %d\n", p->type);
Expand Down Expand Up @@ -450,15 +452,17 @@ int unpack_schema_change_protobuf(struct schema_change_type *s, void *packed_sc,
break;
}
case PARTITION_ADD_COL_HASH: {
strncpy(s->partition.u.hash.viewname,sc->hashviewname, sizeof(s->partition.u.hash.viewname));
s->partition.u.hash.viewname = strdup(sc->hashviewname);
s->partition.u.hash.num_partitions = sc->hashnumpartitions;
s->partition.u.hash.num_columns = sc->hashnumcolumns;
s->partition.u.hash.partitions = (char **)malloc(sizeof(char *) * s->partition.u.hash.num_partitions);
s->partition.u.hash.columns = (char **)malloc(sizeof(char *) * s->partition.u.hash.num_columns);
for (int i=0;i < sc->n_hashcolumns; i++) {
strncpy(s->partition.u.hash.columns[i], sc->hashcolumns[i], sizeof(s->partition.u.hash.columns[i]));
s->partition.u.hash.columns[i] = strdup(sc->hashcolumns[i]);
}

for(int i=0; i<sc->n_hashpartitions;i++) {
strncpy(s->partition.u.hash.partitions[i], sc->hashpartitions[i], sizeof(s->partition.u.hash.partitions[i]));
s->partition.u.hash.partitions[i] = strdup(sc->hashpartitions[i]);
}
s->partition.u.hash.createQuery = strdup(sc->hashcreatequery);
break;
Expand Down Expand Up @@ -487,7 +491,7 @@ void *buf_get_schemachange_protobuf(struct schema_change_type *s, void *p_buf, v

void *buf_put_schemachange(struct schema_change_type *s, void *p_buf, void *p_buf_end)
{

size_t len = 0;
if (p_buf >= p_buf_end) return NULL;

p_buf = buf_put(&s->kind, sizeof(s->kind), p_buf, p_buf_end);
Expand Down Expand Up @@ -614,16 +618,22 @@ void *buf_put_schemachange(struct schema_change_type *s, void *p_buf, void *p_bu
break;
}
case PARTITION_ADD_COL_HASH: {
p_buf = buf_no_net_put(s->partition.u.hash.viewname, sizeof(s->partition.u.hash.viewname), p_buf, p_buf_end);
len = strlen(s->partition.u.hash.viewname);
p_buf = buf_put(&len, sizeof(len), p_buf, p_buf_end);
p_buf = buf_no_net_put(s->partition.u.hash.viewname, strlen(s->partition.u.hash.viewname), p_buf, p_buf_end);
p_buf = buf_put(&s->partition.u.hash.num_columns, sizeof(s->partition.u.hash.num_columns), p_buf, p_buf_end);
for (int i = 0; i < s->partition.u.hash.num_columns; i++) {
len = strlen(s->partition.u.hash.columns[i]);
p_buf = buf_put(&len, sizeof(len), p_buf, p_buf_end);
p_buf =
buf_no_net_put(s->partition.u.hash.columns[i], sizeof(s->partition.u.hash.columns[i]), p_buf, p_buf_end);
buf_no_net_put(s->partition.u.hash.columns[i], strlen(s->partition.u.hash.columns[i]), p_buf, p_buf_end);
}
p_buf = buf_put(&s->partition.u.hash.num_partitions, sizeof(s->partition.u.hash.num_partitions), p_buf, p_buf_end);
for (int i = 0; i < s->partition.u.hash.num_partitions; i++) {
len = strlen(s->partition.u.hash.partitions[i]);
p_buf = buf_put(&len, sizeof(len), p_buf, p_buf_end);
p_buf =
buf_no_net_put(s->partition.u.hash.partitions[i], sizeof(s->partition.u.hash.partitions[i]), p_buf, p_buf_end);
buf_no_net_put(s->partition.u.hash.partitions[i], strlen(s->partition.u.hash.partitions[i]), p_buf, p_buf_end);
}
break;
}
Expand Down Expand Up @@ -877,7 +887,7 @@ void *buf_get_schemachange_v1(struct schema_change_type *s, void *p_buf,
void *buf_get_schemachange_v2(struct schema_change_type *s,
void *p_buf, void *p_buf_end)
{

size_t len = 0;
if (p_buf >= p_buf_end) return NULL;

p_buf = (uint8_t *)buf_get(&s->kind, sizeof(s->kind), p_buf, p_buf_end);
Expand Down Expand Up @@ -1046,19 +1056,34 @@ void *buf_get_schemachange_v2(struct schema_change_type *s,
break;
}
case PARTITION_ADD_COL_HASH: {
p_buf = (uint8_t *)buf_no_net_get(s->partition.u.hash.viewname, sizeof(s->partition.u.hash.viewname), p_buf,
p_buf = (uint8_t *)buf_get(&len, sizeof(len), p_buf, p_buf_end);
logmsg(LOGMSG_USER, "GOT LEN AS %ld for viewname\n", len);
s->partition.u.hash.viewname = (char *)malloc(sizeof(char) * len + 1);
p_buf = (uint8_t *)buf_no_net_get(s->partition.u.hash.viewname, len, p_buf,
p_buf_end);
logmsg(LOGMSG_USER, "GOT VIEWNAME AS %s \n", s->partition.u.hash.viewname);
s->partition.u.hash.viewname[len]='\0';
p_buf = (uint8_t *)buf_get(&s->partition.u.hash.num_columns, sizeof(s->partition.u.hash.num_columns), p_buf,
p_buf_end);
s->partition.u.hash.columns = (char **)malloc(sizeof(char *) * s->partition.u.hash.num_columns);
for (int i = 0; i < s->partition.u.hash.num_columns; i++) {
p_buf = (uint8_t *)buf_no_net_get(s->partition.u.hash.columns[i], sizeof(s->partition.u.hash.columns[i]),
p_buf = (uint8_t *)buf_get(&len, sizeof(len), p_buf, p_buf_end);
logmsg(LOGMSG_USER, "GOT LEN AS %ld for column %d\n", len, i);
s->partition.u.hash.columns[i] = (char *)malloc(sizeof(char) * len + 1);
p_buf = (uint8_t *)buf_no_net_get(s->partition.u.hash.columns[i], len,
p_buf, p_buf_end);
logmsg(LOGMSG_USER, "GOT COLUMN AS %s \n", s->partition.u.hash.columns[i]);
}
p_buf =
(uint8_t *)buf_get(&s->partition.u.hash.num_partitions, sizeof(s->partition.u.hash.num_partitions), p_buf, p_buf_end);
s->partition.u.hash.partitions = (char **)malloc(sizeof(char *) * s->partition.u.hash.num_partitions);
for (int i = 0; i < s->partition.u.hash.num_partitions; i++) {
p_buf = (uint8_t *)buf_no_net_get(s->partition.u.hash.partitions[i], sizeof(s->partition.u.hash.partitions[i]), p_buf,
p_buf = (uint8_t *)buf_get(&len, sizeof(len), p_buf, p_buf_end);
logmsg(LOGMSG_USER, "GOT LEN AS %ld for partition %d\n", len, i);
s->partition.u.hash.partitions[i] = (char *)malloc(sizeof(char) * len + 1);
p_buf = (uint8_t *)buf_no_net_get(s->partition.u.hash.partitions[i], len, p_buf,
p_buf_end);
logmsg(LOGMSG_USER, "GOT PARTITION AS %s \n", s->partition.u.hash.partitions[i]);
}
break;
}
Expand Down
7 changes: 3 additions & 4 deletions schemachange/schemachange.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,11 @@ struct comdb2_partition {
int version;
} mergetable;
struct hash {
char viewname[MAXTABLELEN];
char *viewname;
uint32_t num_partitions;
uint32_t num_columns; // in partitoning key
char columns[MAXCOLUMNS][MAXCOLNAME];
uint32_t keys[MAXPARTITIONS];
char partitions[MAXPARTITIONS][MAXPARTITIONLEN];
char **columns; // columns in partitioning key
char **partitions;
char *createQuery;
} hash;
} u;
Expand Down
70 changes: 59 additions & 11 deletions sqlite/src/comdb2build.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5069,7 +5069,7 @@ void comdb2CreateTableEnd(

if (ctx->partition && ctx->partition->type == PARTITION_ADD_COL_HASH) {
char tmp_str[MAXTABLELEN] = {0};
strcpy(ctx->partition->u.hash.viewname, ctx->tablename);
ctx->partition->u.hash.viewname = strdup(ctx->tablename);
strcpy(tmp_str , "hash_");
strcpy(tmp_str + 5, ctx->tablename);
strcpy(sc->tablename, tmp_str);
Expand Down Expand Up @@ -7748,8 +7748,8 @@ void create_default_consumer_sp(Parse *p, char *spname)

}
static int comdb2GetHashPartitionParams(Parse* pParse, IdList *pColumn, IdList *pPartitions,
char cols[][MAXCOLNAME], uint32_t *oNumPartitions, uint32_t *oNumColumns,
char partitions[][MAXPARTITIONLEN])
char ***cols, uint32_t *oNumPartitions, uint32_t *oNumColumns,
char ***partitions)
{
int column_exists = 0;
struct comdb2_column *cur_col;
Expand All @@ -7764,6 +7764,7 @@ static int comdb2GetHashPartitionParams(Parse* pParse, IdList *pColumn, IdList *

/* Copy the sharding key names after asserting their existence in the table*/
*oNumColumns = pColumn->nId;
*cols = (char **)malloc(sizeof(char *) * pColumn->nId);
int i;
for (i=0; i <pColumn->nId;i++) {
column_exists = 0;
Expand All @@ -7780,18 +7781,66 @@ static int comdb2GetHashPartitionParams(Parse* pParse, IdList *pColumn, IdList *
return -1;
} else {
// column exists, copy it
strncpy0(cols[i], pColumn->a[i].zName, strlen(pColumn->a[i].zName) + 1);
*cols[i] = strdup(pColumn->a[i].zName);
}
}

/*Copy the table partition names*/
*oNumPartitions = pPartitions->nId;
*partitions = (char **)malloc(sizeof(char *) * pPartitions->nId);
for(i=0;i<pPartitions->nId;i++) {
strncpy0(partitions[i], pPartitions->a[i].zName, strlen(pPartitions->a[i].zName)+1);
*partitions[i] = strdup(pPartitions->a[i].zName);
}
return 0;
}

struct comdb2_partition *_get_comdb2_hash_partition(Parse *pParse, IdList *pColumn, IdList *pPartitions, int remove) {
struct comdb2_partition *partition;
partition = _get_partition(pParse, 0);
partition->type = PARTITION_ADD_COL_HASH;
int column_exists = 0;
struct comdb2_column *cur_col;
struct comdb2_ddl_context *ctx = pParse->comdb2_ddl_ctx;
if (ctx == 0) {
/* An error must have been set. */
assert(pParse->rc != 0);
return NULL;
}

assert(pColumn!=0 && pColumn->nId>0);

/* Copy the sharding key names after asserting their existence in the table*/
partition->u.hash.num_columns = pColumn->nId;
partition->u.hash.columns = (char **)malloc(sizeof(char *) * pColumn->nId);
int i;
for (i=0; i <pColumn->nId;i++) {
column_exists = 0;
LISTC_FOR_EACH(&ctx->schema->column_list, cur_col, lnk)
{
if ((strcasecmp(pColumn->a[i].zName, cur_col->name) == 0)) {
column_exists = 1;
break;
}
}
if (column_exists==0) {
setError(pParse, SQLITE_MISUSE, comdb2_asprintf("Column %s does not exist",
pColumn->a[i].zName));
return NULL;
} else {
// column exists, copy it
partition->u.hash.columns[i] = strdup(pColumn->a[i].zName);
}
}

/*Copy the table partition names*/
partition->u.hash.num_partitions = pPartitions->nId;
partition->u.hash.partitions = (char **)malloc(sizeof(char *) * pPartitions->nId);
for(i=0;i<pPartitions->nId;i++) {
partition->u.hash.partitions[i] = strdup(pPartitions->a[i].zName);
}
return partition;
}

void comdb2CreateHashPartition(Parse *pParse, IdList *pColumn, IdList *pPartitions, int createPartitions, int createTables)
{
struct comdb2_partition *partition;
Expand All @@ -7800,19 +7849,18 @@ void comdb2CreateHashPartition(Parse *pParse, IdList *pColumn, IdList *pPartitio
return;
}

partition = _get_partition(pParse, 0);
partition = _get_comdb2_hash_partition(pParse,pColumn,pPartitions, 0);
if (!partition)
return;

partition->type = PARTITION_ADD_COL_HASH;
if (comdb2GetHashPartitionParams(pParse, pColumn, pPartitions,
partition->u.hash.columns,
/*if (comdb2GetHashPartitionParams(pParse, pColumn, pPartitions,
&partition->u.hash.columns,
(uint32_t*)&partition->u.hash.num_partitions,
(uint32_t*)&partition->u.hash.num_columns,
partition->u.hash.partitions)) {
&partition->u.hash.partitions)) {
free_ddl_context(pParse);
return;
}
}*/

GET_CLNT;
if (clnt && clnt->sql) {
Expand Down

0 comments on commit 3bce474

Please sign in to comment.