From d2e023e3e5ef888e176292f149d1ba93ba270c94 Mon Sep 17 00:00:00 2001 From: Petteri Aimonen Date: Sun, 11 Jan 2015 19:46:15 +0200 Subject: Bugfixes for oneof support. Fixes crashes / memory leaks when using pointer type fields. Also fixes initialization of which_oneof fields. --- pb.h | 2 +- pb_common.c | 20 ++++++++++---------- pb_decode.c | 17 +++++++++++++---- 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/pb.h b/pb.h index af58dca..fc846c9 100644 --- a/pb.h +++ b/pb.h @@ -514,7 +514,7 @@ struct pb_extension_s { #define PB_ONEOF_POINTER(u, tag, st, m, fd, ltype, ptr) \ {tag, PB_ATYPE_POINTER | PB_HTYPE_ONEOF | ltype, \ fd, pb_delta(st, which_ ## u, u.m), \ - pb_membersize(st, u.m), 0, ptr} + pb_membersize(st, u.m[0]), 0, ptr} #define PB_ONEOF_FIELD(union_name, tag, type, rules, allocation, placement, message, field, prevfield, ptr) \ PB_ ## rules ## _ ## allocation(union_name, tag, message, field, \ diff --git a/pb_common.c b/pb_common.c index 9896485..385c019 100644 --- a/pb_common.c +++ b/pb_common.c @@ -41,8 +41,15 @@ bool pb_field_iter_next(pb_field_iter_t *iter) /* Increment the pointers based on previous field size */ size_t prev_size = prev_field->data_size; - if (PB_ATYPE(prev_field->type) == PB_ATYPE_STATIC && - PB_HTYPE(prev_field->type) == PB_HTYPE_REPEATED) + if (PB_HTYPE(prev_field->type) == PB_HTYPE_ONEOF && + PB_HTYPE(iter->pos->type) == PB_HTYPE_ONEOF) + { + /* Don't advance pointers inside unions */ + prev_size = 0; + iter->pData = (char*)iter->pData - prev_field->data_offset; + } + else if (PB_ATYPE(prev_field->type) == PB_ATYPE_STATIC && + PB_HTYPE(prev_field->type) == PB_HTYPE_REPEATED) { /* In static arrays, the data_size tells the size of a single entry and * array_size is the number of entries */ @@ -54,14 +61,7 @@ bool pb_field_iter_next(pb_field_iter_t *iter) * The data_size only applies to the dynamically allocated area. */ prev_size = sizeof(void*); } - else if (PB_HTYPE(prev_field->type) == PB_HTYPE_ONEOF && - PB_HTYPE(iter->pos->type) == PB_HTYPE_ONEOF) - { - /* Don't advance pointers inside unions */ - prev_size = 0; - iter->pData = (char*)iter->pData - prev_field->data_offset; - } - + if (PB_HTYPE(prev_field->type) == PB_HTYPE_REQUIRED) { /* Count the required fields, in order to check their presence in the diff --git a/pb_decode.c b/pb_decode.c index 542fdc4..0677594 100644 --- a/pb_decode.c +++ b/pb_decode.c @@ -747,13 +747,15 @@ static void pb_field_set_to_default(pb_field_iter_t *iter) * itself also. */ *(bool*)iter->pSize = false; } - else if (PB_HTYPE(type) == PB_HTYPE_REPEATED) + else if (PB_HTYPE(type) == PB_HTYPE_REPEATED || + PB_HTYPE(type) == PB_HTYPE_ONEOF) { - /* Set array count to 0, no need to initialize contents. */ + /* REPEATED: Set array count to 0, no need to initialize contents. + ONEOF: Set which_field to 0. */ *(pb_size_t*)iter->pSize = 0; init_data = false; } - + if (init_data) { if (PB_LTYPE(iter->pos->type) == PB_LTYPE_SUBMESSAGE) @@ -779,7 +781,8 @@ static void pb_field_set_to_default(pb_field_iter_t *iter) *(void**)iter->pData = NULL; /* Initialize array count to 0. */ - if (PB_HTYPE(type) == PB_HTYPE_REPEATED) + if (PB_HTYPE(type) == PB_HTYPE_REPEATED || + PB_HTYPE(type) == PB_HTYPE_ONEOF) { *(pb_size_t*)iter->pSize = 0; } @@ -938,6 +941,12 @@ static void pb_release_single_field(const pb_field_iter_t *iter) pb_type_t type; type = iter->pos->type; + if (PB_HTYPE(type) == PB_HTYPE_ONEOF) + { + if (*(pb_size_t*)iter->pSize != iter->pos->tag) + return; /* This is not the current field in the union */ + } + /* Release anything contained inside an extension or submsg. * This has to be done even if the submsg itself is statically * allocated. */ -- cgit v1.2.3