From 1aafd4111b9b6d08d2d0937b0f396a4caa9ea04d Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Sat, 8 Jul 2017 00:00:05 -0700 Subject: A good start on upb_encode and upb_decode. --- tests/conformance_upb.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) (limited to 'tests/conformance_upb.c') diff --git a/tests/conformance_upb.c b/tests/conformance_upb.c index 9f83e80..e1221b2 100644 --- a/tests/conformance_upb.c +++ b/tests/conformance_upb.c @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -52,7 +53,7 @@ void DoTest( if (!test_message) { /* TODO(haberman): return details. */ - static char msg[] = "Parse error (no more details available)."; + static const char msg[] = "Parse error (no more details available)."; conformance_ConformanceResponse_set_parse_error( response, upb_stringview_make(msg, sizeof(msg))); return; @@ -60,20 +61,20 @@ void DoTest( break; case conformance_ConformanceRequest_payload_json_payload: { - static char msg[] = "JSON support not yet implemented."; + static const char msg[] = "JSON support not yet implemented."; conformance_ConformanceResponse_set_skipped( response, upb_stringview_make(msg, sizeof(msg))); return; } case conformance_ConformanceRequest_payload_NOT_SET: - fprintf(stderr, "conformance_upb: Request didn't have payload."); - exit(1); + fprintf(stderr, "conformance_upb: Request didn't have payload.\n"); + return; } switch (conformance_ConformanceRequest_requested_output_format(request)) { case conformance_UNSPECIFIED: - fprintf(stderr, "conformance_upb: Unspecified output format."); + fprintf(stderr, "conformance_upb: Unspecified output format.\n"); exit(1); case conformance_PROTOBUF: { @@ -81,8 +82,10 @@ void DoTest( char *serialized = protobuf_test_messages_proto3_TestAllTypes_serialize( test_message, env, &serialized_len); if (!serialized) { - fprintf(stderr, "conformance_upb: Error serialiing."); - exit(1); + static const char msg[] = "Error serializing."; + conformance_ConformanceResponse_set_serialize_error( + response, upb_stringview_make(msg, sizeof(msg))); + return; } conformance_ConformanceResponse_set_protobuf_payload( response, upb_stringview_make(serialized, serialized_len)); @@ -90,14 +93,14 @@ void DoTest( } case conformance_JSON: { - static char msg[] = "JSON support not yet implemented."; + static const char msg[] = "JSON support not yet implemented."; conformance_ConformanceResponse_set_skipped( response, upb_stringview_make(msg, sizeof(msg))); break; } default: - fprintf(stderr, "conformance_upb: Unknown output format: %d", + fprintf(stderr, "conformance_upb: Unknown output format: %d\n", conformance_ConformanceRequest_requested_output_format(request)); exit(1); } @@ -111,7 +114,7 @@ bool DoTestIo() { char *serialized_input; char *serialized_output; uint32_t input_size; - size_t output_size; + size_t output_size = 0; conformance_ConformanceRequest *request; conformance_ConformanceResponse *response; -- cgit v1.2.3 From 806ffc1d2053f1c02167c7965b39abc997d12ad6 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Tue, 18 Jul 2017 11:48:29 +0200 Subject: Responded to PR comments. --- tests/conformance_upb.c | 2 +- tools/make_c_api.lua | 8 +++----- upb/decode.c | 9 +++++---- upb/encode.c | 8 ++++++-- 4 files changed, 15 insertions(+), 12 deletions(-) (limited to 'tests/conformance_upb.c') diff --git a/tests/conformance_upb.c b/tests/conformance_upb.c index e1221b2..2ef8c6d 100644 --- a/tests/conformance_upb.c +++ b/tests/conformance_upb.c @@ -114,7 +114,7 @@ bool DoTestIo() { char *serialized_input; char *serialized_output; uint32_t input_size; - size_t output_size = 0; + size_t output_size; conformance_ConformanceRequest *request; conformance_ConformanceResponse *response; diff --git a/tools/make_c_api.lua b/tools/make_c_api.lua index cfcf38d..552e861 100644 --- a/tools/make_c_api.lua +++ b/tools/make_c_api.lua @@ -155,6 +155,8 @@ local function field_layout_rank(field) -- 1. padding alignment is (nearly) minimized. -- 2. fields that might have defaults (1-4) are segregated -- from fields that are always zero-initialized (5-7). + -- + -- We skip oneof fields, because they are emitted in a separate pass. local rank if field:containing_oneof() then rank = 100 -- These go last (actually we skip them). @@ -347,7 +349,7 @@ local function write_c_file(filedef, hfilename, append) end if field:containing_oneof() then - -- Do nothing now + -- Handled below. else if has_hasbit(field) then hasbit_indexes[field] = hasbit_count @@ -358,14 +360,11 @@ local function write_c_file(filedef, hfilename, append) end end - local oneof_last_fields = {} -- Oneof fields. for oneof in msg:oneofs() do local fullname = to_cident(oneof:containing_type():full_name() .. "." .. oneof:name()) append(' union {\n') - oneof_last_fields[oneof] = "" for field in oneof:fields() do - oneof_last_fields[oneof] = field:name() append(' %s %s;\n', ctype(field), field:name()) end append(' } %s;\n', oneof:name()) @@ -425,7 +424,6 @@ local function write_c_file(filedef, hfilename, append) if field:containing_oneof() then oneof_index = oneof_indexes[field:containing_oneof()] end - -- TODO(haberman): oneofs. append(' {%s, offsetof(%s, %s), %s, %s, %s, %s, %s},\n', field:number(), msgname, diff --git a/upb/decode.c b/upb/decode.c index 3a44021..b09da42 100644 --- a/upb/decode.c +++ b/upb/decode.c @@ -54,18 +54,18 @@ static bool upb_decode_message(upb_decstate *d, const char *limit, static bool upb_decode_varint(const char **ptr, const char *limit, uint64_t *val) { - uint8_t byte = 0x80; + uint8_t byte; int bitpos = 0; const char *p = *ptr; *val = 0; - while (byte & 0x80) { + do { CHK(bitpos < 70 && p < limit); byte = *p; *val |= (uint64_t)(byte & 0x7F) << bitpos; p++; bitpos += 7; - } + } while (byte & 0x80); *ptr = p; return true; @@ -385,7 +385,7 @@ static bool upb_decode_fixedpacked(upb_array *arr, upb_stringview data, int elements = data.size / elem_size; void *field_mem; - CHK((data.size % elem_size) == 0); + CHK(elements * elem_size == data.size); field_mem = upb_array_add(arr, elements); CHK(field_mem); memcpy(field_mem, data.data, data.size); @@ -451,6 +451,7 @@ static bool upb_decode_toarray(upb_decstate *d, upb_decframe *frame, return upb_append_unknown(d, frame, field_start); } #undef VARINT_CASE + UPB_UNREACHABLE(); } static bool upb_decode_delimitedfield(upb_decstate *d, upb_decframe *frame, diff --git a/upb/encode.c b/upb/encode.c index 4aafb9c..e7585eb 100644 --- a/upb/encode.c +++ b/upb/encode.c @@ -32,7 +32,7 @@ static const uint8_t upb_desctype_to_fieldtype[] = { static size_t upb_encode_varint(uint64_t val, char *buf) { size_t i; - if (val == 0) { buf[0] = 0; return 1; } + if (val < 128) { buf[0] = val; return 1; } i = 0; while (val) { uint8_t byte = val & 0x7fU; @@ -373,7 +373,11 @@ char *upb_encode(const void *msg, const upb_msglayout_msginit_v1 *m, e.limit = NULL; e.ptr = NULL; - CHK(upb_encode_message(&e, msg, m, size)); + if (!upb_encode_message(&e, msg, m, size)) { + *size = 0; + return NULL; + } + *size = e.limit - e.ptr; if (*size == 0) { -- cgit v1.2.3 From 6b8767422154008eed98d0df42e36758d38877a4 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Wed, 19 Jul 2017 00:33:30 +0200 Subject: Address review comments and fix compile warnings. --- tests/conformance_upb.c | 2 +- upb/decode.c | 2 +- upb/encode.c | 10 ++++++---- 3 files changed, 8 insertions(+), 6 deletions(-) (limited to 'tests/conformance_upb.c') diff --git a/tests/conformance_upb.c b/tests/conformance_upb.c index 2ef8c6d..70ea300 100644 --- a/tests/conformance_upb.c +++ b/tests/conformance_upb.c @@ -33,7 +33,7 @@ bool CheckedRead(int fd, void *buf, size_t len) { } void CheckedWrite(int fd, const void *buf, size_t len) { - if (write(fd, buf, len) != len) { + if ((size_t)write(fd, buf, len) != len) { perror("writing to test runner"); exit(1); } diff --git a/upb/decode.c b/upb/decode.c index b09da42..cba9f61 100644 --- a/upb/decode.c +++ b/upb/decode.c @@ -385,7 +385,7 @@ static bool upb_decode_fixedpacked(upb_array *arr, upb_stringview data, int elements = data.size / elem_size; void *field_mem; - CHK(elements * elem_size == data.size); + CHK((size_t)(elements * elem_size) == data.size); field_mem = upb_array_add(arr, elements); CHK(field_mem); memcpy(field_mem, data.data, data.size); diff --git a/upb/encode.c b/upb/encode.c index e7585eb..cb72bf9 100644 --- a/upb/encode.c +++ b/upb/encode.c @@ -256,9 +256,11 @@ static bool upb_encode_scalarfield(upb_encstate *e, const char *field_mem, const upb_msglayout_msginit_v1 *m, const upb_msglayout_fieldinit_v1 *f, bool is_proto3) { + bool skip_zero_value = is_proto3 && f->oneof_index == UPB_NOT_IN_ONEOF; + #define CASE(ctype, type, wire_type, encodeval) do { \ ctype val = *(ctype*)field_mem; \ - if (is_proto3 && f->oneof_index == UPB_NOT_IN_ONEOF && val == 0) { \ + if (skip_zero_value && val == 0) { \ return true; \ } \ return upb_put_ ## type(e, encodeval) && \ @@ -292,7 +294,7 @@ static bool upb_encode_scalarfield(upb_encstate *e, const char *field_mem, case UPB_DESCRIPTOR_TYPE_STRING: case UPB_DESCRIPTOR_TYPE_BYTES: { upb_stringview view = *(upb_stringview*)field_mem; - if (is_proto3 && f->oneof_index == UPB_NOT_IN_ONEOF && view.size == 0) { + if (skip_zero_value && view.size == 0) { return true; } return upb_put_bytes(e, view.data, view.size) && @@ -303,7 +305,7 @@ static bool upb_encode_scalarfield(upb_encstate *e, const char *field_mem, size_t size; void *submsg = *(void**)field_mem; const upb_msglayout_msginit_v1 *subm = m->submsgs[f->submsg_index]; - if (is_proto3 && f->oneof_index == UPB_NOT_IN_ONEOF && submsg == NULL) { + if (skip_zero_value && submsg == NULL) { return true; } return upb_put_tag(e, f->number, UPB_WIRE_TYPE_END_GROUP) && @@ -314,7 +316,7 @@ static bool upb_encode_scalarfield(upb_encstate *e, const char *field_mem, size_t size; void *submsg = *(void**)field_mem; const upb_msglayout_msginit_v1 *subm = m->submsgs[f->submsg_index]; - if (is_proto3 && f->oneof_index == UPB_NOT_IN_ONEOF && submsg == NULL) { + if (skip_zero_value && submsg == NULL) { return true; } return upb_encode_message(e, submsg, subm, &size) && -- cgit v1.2.3