From dda1499a0eab41165c82277e630dd7050145448f Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Tue, 19 Jan 2010 10:28:51 -0800 Subject: [PATCH] Add status to the sink interfaces. --- src/upb_data.c | 64 +++++++++++++++++++++++++-------- src/upb_data.h | 10 ++++-- src/upb_decoder.c | 16 ++++----- src/upb_encoder.c | 91 ++++++++++++++++++++++++++++++----------------- src/upb_encoder.h | 12 ++++--- src/upb_sink.h | 27 ++++++++------ 6 files changed, 146 insertions(+), 74 deletions(-) diff --git a/src/upb_data.c b/src/upb_data.c index 8ed1646e8a..d5a631d63d 100644 --- a/src/upb_data.c +++ b/src/upb_data.c @@ -297,25 +297,46 @@ void upb_msg_decodestr(upb_msg *msg, upb_msgdef *md, upb_strptr str, upb_msgsink_free(s); } +#if 0 +void upb_msg_encodestr(upb_msg *msg, upb_msgdef *md, upb_strptr str, + upb_status *status) +{ + upb_sizebuilder *sb = upb_sizebuilder_new(md); + upb_encoder *e = upb_encoder_new(md); + upb_strsink *sink = upb_strsink_new(); + + // Get sizes. We could avoid performing this step in some cases by having a + // bool in the msgdef indicating whether it or any of its children have + // submessages in the def (groups don't count). + upb_sizebuilder_reset(sb); + upb_msgsrc_produce(msg, md, upb_sizebuilder_sink(sb), true); + + upb_strsink_reset(); + upb_encoder_reset(e, sb, sink); + upb_msgsrc_produce(msg, md, sink, false); +} +#endif /* upb_msgsrc ****************************************************************/ static void _upb_msgsrc_produceval(upb_value v, upb_fielddef *f, upb_sink *sink, - bool reverse) + bool reverse, upb_status *status) { + // TODO: We need to check status for failure, but how often? if(upb_issubmsg(f)) { - upb_sink_onstart(sink, f); - upb_msgsrc_produce(v.msg, upb_downcast_msgdef(f->def), sink, reverse); - upb_sink_onend(sink, f); + upb_msgdef *md = upb_downcast_msgdef(f->def); + upb_sink_onstart(sink, f, status); + upb_msgsrc_produce(v.msg, md, sink, reverse, status); + upb_sink_onend(sink, f, status); } else if(upb_isstring(f)) { - upb_sink_onstr(sink, f, v.str, 0, upb_strlen(v.str)); + upb_sink_onstr(sink, f, v.str, 0, upb_strlen(v.str), status); } else { - upb_sink_onvalue(sink, f, v); + upb_sink_onvalue(sink, f, v, status); } } void upb_msgsrc_produce(upb_msg *msg, upb_msgdef *md, upb_sink *sink, - bool reverse) + bool reverse, upb_status *status) { for(int i = 0; i < md->num_fields; i++) { upb_fielddef *f = &md->fields[reverse ? md->num_fields - i - 1 : i]; @@ -326,10 +347,10 @@ void upb_msgsrc_produce(upb_msg *msg, upb_msgdef *md, upb_sink *sink, upb_arraylen_t len = upb_array_len(arr); for(upb_arraylen_t j = 0; j < upb_array_len(arr); j++) { upb_value elem = upb_array_get(arr, f, reverse ? len - j - 1 : j); - _upb_msgsrc_produceval(elem, f, sink, reverse); + _upb_msgsrc_produceval(elem, f, sink, reverse, status); } } else { - _upb_msgsrc_produceval(v, f, sink, reverse); + _upb_msgsrc_produceval(v, f, sink, reverse, status); } } } @@ -345,7 +366,7 @@ typedef struct { struct upb_msgsink { upb_sink base; upb_msgdef *toplevel_msgdef; - upb_msgsink_frame stack[UPB_MAX_NESTING], *top; + upb_msgsink_frame stack[UPB_MAX_NESTING], *top, *limit; }; /* Helper function that returns a pointer to where the next value for field "f" @@ -377,8 +398,9 @@ static upb_valueptr get_valueptr(upb_msg *msg, upb_fielddef *f) // TODO: implement these in terms of public interfaces. static upb_sink_status _upb_msgsink_valuecb(upb_sink *s, upb_fielddef *f, - upb_value val) + upb_value val, upb_status *status) { + (void)status; // No detectable errors can occur. upb_msgsink *ms = (upb_msgsink*)s; upb_msg *msg = ms->top->msg; upb_valueptr p = get_valueptr(msg, f); @@ -389,8 +411,10 @@ static upb_sink_status _upb_msgsink_valuecb(upb_sink *s, upb_fielddef *f, static upb_sink_status _upb_msgsink_strcb(upb_sink *s, upb_fielddef *f, upb_strptr str, - int32_t start, uint32_t end) + int32_t start, uint32_t end, + upb_status *status) { + (void)status; // No detectable errors can occur. upb_msgsink *ms = (upb_msgsink*)s; upb_msg *msg = ms->top->msg; upb_valueptr p = get_valueptr(msg, f); @@ -405,11 +429,19 @@ static upb_sink_status _upb_msgsink_strcb(upb_sink *s, upb_fielddef *f, return UPB_SINK_CONTINUE; } -static upb_sink_status _upb_msgsink_startcb(upb_sink *s, upb_fielddef *f) +static upb_sink_status _upb_msgsink_startcb(upb_sink *s, upb_fielddef *f, + upb_status *status) { upb_msgsink *ms = (upb_msgsink*)s; upb_msg *oldmsg = ms->top->msg; upb_valueptr p = get_valueptr(oldmsg, f); + ms->top++; + if(ms->top == ms->limit) { + upb_seterr(status, UPB_ERROR_MAX_NESTING_EXCEEDED, + "Nesting exceeded maximum (%d levels)\n", + UPB_MAX_NESTING); + return UPB_SINK_STOP; + } if(upb_isarray(f) || !upb_msg_has(oldmsg, f)) { upb_msgdef *md = upb_downcast_msgdef(f->def); @@ -422,13 +454,14 @@ static upb_sink_status _upb_msgsink_startcb(upb_sink *s, upb_fielddef *f) upb_msg_sethas(oldmsg, f); } - ms->top++; ms->top->msg = *p.msg; return UPB_SINK_CONTINUE; } -static upb_sink_status _upb_msgsink_endcb(upb_sink *s, upb_fielddef *f) +static upb_sink_status _upb_msgsink_endcb(upb_sink *s, upb_fielddef *f, + upb_status *status) { + (void)status; // No detectable errors can occur. (void)f; // Unused. upb_msgsink *ms = (upb_msgsink*)s; ms->top--; @@ -467,6 +500,7 @@ upb_sink *upb_msgsink_sink(upb_msgsink *sink) void upb_msgsink_reset(upb_msgsink *ms, upb_msg *msg) { ms->top = ms->stack; + ms->limit = ms->stack + UPB_MAX_NESTING; ms->top->msg = msg; ms->top->md = ms->toplevel_msgdef; } diff --git a/src/upb_data.h b/src/upb_data.h index 2d2f40232e..cdb7af2133 100644 --- a/src/upb_data.h +++ b/src/upb_data.h @@ -509,18 +509,24 @@ INLINE void upb_msg_clear(upb_msg *msg, upb_msgdef *md) { memset(msg->data, 0, md->set_flags_bytes); } -// A convenience function for parsing an entire protobuf all at once, without +// A convenience function for decoding an entire protobuf all at once, without // having to worry about setting up the appropriate objects. void upb_msg_decodestr(upb_msg *msg, upb_msgdef *md, upb_strptr str, upb_status *status); +// A convenience function for encoding an entire protobuf all at once. If an +// error occurs, the null string is returned and the status object contains +// the error. +void upb_msg_encodestr(upb_msg *msg, upb_msgdef *md, upb_strptr str, + upb_status *status); + /* upb_msgsrc *****************************************************************/ // A nonresumable, non-interruptable (but simple and fast) source for pushing // the data of a upb_msg to a upb_sink. void upb_msgsrc_produce(upb_msg *msg, upb_msgdef *md, upb_sink *sink, - bool reverse); + bool reverse, upb_status *status); /* upb_msgsink ****************************************************************/ diff --git a/src/upb_decoder.c b/src/upb_decoder.c index 440eaefb33..209db566cf 100644 --- a/src/upb_decoder.c +++ b/src/upb_decoder.c @@ -383,16 +383,16 @@ static const uint8_t *push(upb_decoder *d, const uint8_t *start, frame->end_offset = d->completed_offset + submsg_len; frame->msgdef = upb_downcast_msgdef(f->def); - upb_sink_onstart(d->sink, f); + upb_sink_onstart(d->sink, f, status); return get_msgend(d, start); } // Pops a stack frame, returning a pointer for where the next submsg should // end (or a pointer that is out of range for a group). -static const void *pop(upb_decoder *d, const uint8_t *start) +static const void *pop(upb_decoder *d, const uint8_t *start, upb_status *status) { d->top--; - upb_sink_onend(d->sink, d->top->field); + upb_sink_onend(d->sink, d->top->field, status); return get_msgend(d, start); } @@ -430,7 +430,7 @@ size_t upb_decoder_decode(upb_decoder *d, upb_strptr str, upb_status *status) d->completed_offset + (completed - start)); goto err; } - submsg_end = pop(d, start); + submsg_end = pop(d, start, status); msgdef = d->top->msgdef; completed = buf; continue; @@ -451,8 +451,8 @@ size_t upb_decoder_decode(upb_decoder *d, upb_strptr str, upb_status *status) } else { if(f && upb_isstringtype(f->type)) { int32_t str_start = buf - start; - sink_status = - upb_sink_onstr(d->sink, f, str, str_start, str_start + delim_len); + uint32_t len = str_start + delim_len; + sink_status = upb_sink_onstr(d->sink, f, str, str_start, len, status); } // else { TODO: packed arrays } // If field was not found, it is skipped silently. buf = delim_end; // Could be >end. @@ -468,7 +468,7 @@ size_t upb_decoder_decode(upb_decoder *d, upb_strptr str, upb_status *status) buf = upb_decode_value(buf, end, f->type, upb_value_addrof(&val), status); CHECK_STATUS(); // Checking upb_decode_value(). - sink_status = upb_sink_onvalue(d->sink, f, val); + sink_status = upb_sink_onvalue(d->sink, f, val, status); } } CHECK_STATUS(); @@ -479,7 +479,7 @@ size_t upb_decoder_decode(upb_decoder *d, upb_strptr str, upb_status *status) "did not lie on a tag/value boundary."); goto err; } - submsg_end = pop(d, start); + submsg_end = pop(d, start, status); msgdef = d->top->msgdef; } // while(buf < d->packed_end) { TODO: packed arrays } diff --git a/src/upb_encoder.c b/src/upb_encoder.c index 6d57accb64..048b2b7b58 100644 --- a/src/upb_encoder.c +++ b/src/upb_encoder.c @@ -248,8 +248,10 @@ struct upb_sizebuilder { // upb_sink callbacks. static upb_sink_status _upb_sizebuilder_valuecb(upb_sink *sink, upb_fielddef *f, - upb_value val) + upb_value val, + upb_status *status) { + (void)status; upb_sizebuilder *sb = (upb_sizebuilder*)sink; uint32_t size = 0; size += _upb_get_tag_size(f->number); @@ -260,8 +262,10 @@ static upb_sink_status _upb_sizebuilder_valuecb(upb_sink *sink, upb_fielddef *f, static upb_sink_status _upb_sizebuilder_strcb(upb_sink *sink, upb_fielddef *f, upb_strptr str, - int32_t start, uint32_t end) + int32_t start, uint32_t end, + upb_status *status) { + (void)status; (void)str; // String data itself is not used. upb_sizebuilder *sb = (upb_sizebuilder*)sink; if(start >= 0) { @@ -273,37 +277,54 @@ static upb_sink_status _upb_sizebuilder_strcb(upb_sink *sink, upb_fielddef *f, return UPB_SINK_CONTINUE; } -static upb_sink_status _upb_sizebuilder_startcb(upb_sink *sink, upb_fielddef *f) +static upb_sink_status _upb_sizebuilder_startcb(upb_sink *sink, upb_fielddef *f, + upb_status *status) { (void)f; // Unused (we calculate tag size and delimiter in endcb). upb_sizebuilder *sb = (upb_sizebuilder*)sink; - *sb->top = sb->size; - sb->top++; - sb->size = 0; - if(sb->top == sb->limit) { - upb_seterr(&sb->status, UPB_ERROR_MAX_NESTING_EXCEEDED, - "Nesting exceeded maximum (%d levels)\n", - UPB_MAX_NESTING); - return UPB_SINK_STOP; + if(f->type == UPB_TYPE(MESSAGE)) { + *sb->top = sb->size; + sb->top++; + sb->size = 0; + if(sb->top == sb->limit) { + upb_seterr(status, UPB_ERROR_MAX_NESTING_EXCEEDED, + "Nesting exceeded maximum (%d levels)\n", + UPB_MAX_NESTING); + return UPB_SINK_STOP; + } + } else { + assert(f->type == UPB_TYPE(GROUP)); + sb->size += _upb_get_tag_size(f->number); } return UPB_SINK_CONTINUE; } -static upb_sink_status _upb_sizebuilder_endcb(upb_sink *sink, upb_fielddef *f) +static upb_sink_status _upb_sizebuilder_endcb(upb_sink *sink, upb_fielddef *f, + upb_status *status) { + (void)status; upb_sizebuilder *sb = (upb_sizebuilder*)sink; - if(sb->sizes_len == sb->sizes_size) { - sb->sizes_size *= 2; - sb->sizes = realloc(sb->sizes, sb->sizes_size * sizeof(*sb->sizes)); + if(f->type == UPB_TYPE(MESSAGE)) { + sb->top--; + if(sb->sizes_len == sb->sizes_size) { + sb->sizes_size *= 2; + sb->sizes = realloc(sb->sizes, sb->sizes_size * sizeof(*sb->sizes)); + } + uint32_t child_size = sb->size; + uint32_t parent_size = *sb->top; + sb->sizes[sb->sizes_len++] = child_size; + // The size according to the parent includes the tag size and delimiter of + // the submessage. + parent_size += upb_get_UINT32_size(child_size); + parent_size += _upb_get_tag_size(f->number); + // Include size accumulated in parent before child began. + sb->size = child_size + parent_size; + } else { + assert(f->type == UPB_TYPE(GROUP)); + // As an optimization, we could just add this number twice in startcb, to + // avoid having to recalculate it. + sb->size += _upb_get_tag_size(f->number); } - sb->sizes[sb->sizes_len++] = sb->size; - sb->top--; - // The size according to the parent includes the tag size and delimiter of - // the submessage. - sb->size += upb_get_UINT32_size(sb->size); - sb->size += _upb_get_tag_size(f->number); - // Include size accumulated in parent before child began. - sb->size += *sb->top; return UPB_SINK_CONTINUE; } @@ -329,12 +350,13 @@ struct upb_encoder { #define UPB_ENCODER_BUFSIZE (UPB_MAX_ENCODED_SIZE * 2) static upb_sink_status _upb_encoder_push_buf(upb_encoder *s, const uint8_t *buf, - size_t len) + size_t len, upb_status *status) { // TODO: conjure a upb_strptr that points to buf. //upb_strptr ptr; (void)s; (void)buf; + (void)status; size_t written = 5;// = upb_bytesink_onbytes(s->bytesink, ptr); if(written < len) { // TODO: mark to skip "written" bytes next time. @@ -345,7 +367,7 @@ static upb_sink_status _upb_encoder_push_buf(upb_encoder *s, const uint8_t *buf, } static upb_sink_status _upb_encoder_valuecb(upb_sink *sink, upb_fielddef *f, - upb_value val) + upb_value val, upb_status *status) { upb_encoder *s = (upb_encoder*)sink; uint8_t buf[UPB_ENCODER_BUFSIZE], *ptr = buf; @@ -353,12 +375,13 @@ static upb_sink_status _upb_encoder_valuecb(upb_sink *sink, upb_fielddef *f, // TODO: handle packed encoding. ptr = _upb_put_tag(ptr, f->number, wt); ptr = upb_encode_value(ptr, f->type, val); - return _upb_encoder_push_buf(s, buf, ptr - buf); + return _upb_encoder_push_buf(s, buf, ptr - buf, status); } static upb_sink_status _upb_encoder_strcb(upb_sink *sink, upb_fielddef *f, upb_strptr str, - int32_t start, uint32_t end) + int32_t start, uint32_t end, + upb_status *status) { upb_encoder *s = (upb_encoder*)sink; uint8_t buf[UPB_ENCODER_BUFSIZE], *ptr = buf; @@ -368,11 +391,12 @@ static upb_sink_status _upb_encoder_strcb(upb_sink *sink, upb_fielddef *f, } // TODO: properly handle partially consumed strings and partially supplied // strings. - _upb_encoder_push_buf(s, buf, ptr - buf); - return _upb_encoder_push_buf(s, (uint8_t*)upb_string_getrobuf(str), end - start); + _upb_encoder_push_buf(s, buf, ptr - buf, status); + return _upb_encoder_push_buf(s, (uint8_t*)upb_string_getrobuf(str), end - start, status); } -static upb_sink_status _upb_encoder_startcb(upb_sink *sink, upb_fielddef *f) +static upb_sink_status _upb_encoder_startcb(upb_sink *sink, upb_fielddef *f, + upb_status *status) { upb_encoder *s = (upb_encoder*)sink; uint8_t buf[UPB_ENCODER_BUFSIZE], *ptr = buf; @@ -382,16 +406,17 @@ static upb_sink_status _upb_encoder_startcb(upb_sink *sink, upb_fielddef *f) ptr = _upb_put_tag(ptr, f->number, UPB_WIRE_TYPE_DELIMITED); ptr = upb_put_UINT32(ptr, s->sizes[--s->size_offset]); } - return _upb_encoder_push_buf(s, buf, ptr - buf); + return _upb_encoder_push_buf(s, buf, ptr - buf, status); } -static upb_sink_status _upb_encoder_endcb(upb_sink *sink, upb_fielddef *f) +static upb_sink_status _upb_encoder_endcb(upb_sink *sink, upb_fielddef *f, + upb_status *status) { upb_encoder *s = (upb_encoder*)sink; uint8_t buf[UPB_ENCODER_BUFSIZE], *ptr = buf; if(f->type != UPB_TYPE(GROUP)) return UPB_SINK_CONTINUE; ptr = _upb_put_tag(ptr, f->number, UPB_WIRE_TYPE_END_GROUP); - return _upb_encoder_push_buf(s, buf, ptr - buf); + return _upb_encoder_push_buf(s, buf, ptr - buf, status); } upb_sink_callbacks _upb_encoder_sink_vtbl = { diff --git a/src/upb_encoder.h b/src/upb_encoder.h index 1137c7f590..b4d0c9839a 100644 --- a/src/upb_encoder.h +++ b/src/upb_encoder.h @@ -29,9 +29,11 @@ extern "C" { struct upb_sizebuilder; typedef struct upb_sizebuilder upb_sizebuilder; -upb_sizebuilder *upb_sizebuilder_new(); +upb_sizebuilder *upb_sizebuilder_new(upb_msgdef *md); void upb_sizebuilder_free(upb_sizebuilder *sb); +void upb_sizebuilder_reset(upb_sizebuilder *sb); + // Returns a sink that must be used to perform the pre-pass. Note that the // pre-pass *must* occur in the opposite order from the actual encode that // follows, and the data *must* be identical both times (except for the @@ -46,8 +48,8 @@ upb_sink *upb_sizebuilder_sink(upb_sizebuilder *sb); struct upb_encoder; typedef struct upb_encoder upb_encoder; -upb_encoder *upb_encoder_new(); -void upb_encoder_free(upb_encoder *s); +upb_encoder *upb_encoder_new(upb_msgdef *md); +void upb_encoder_free(upb_encoder *e); // Resets the given upb_encoder such that is is ready to begin encoding. The // upb_sizebuilder "sb" is used to determine submessage sizes; it must have @@ -57,12 +59,12 @@ void upb_encoder_free(upb_encoder *s); // "out" is where the encoded output data will be sent. // // Both "sb" and "out" must live until the encoder is either reset or freed. -void upb_encoder_reset(upb_encoder *s, upb_sizebuilder *sb, upb_bytesink *out); +void upb_encoder_reset(upb_encoder *e, upb_sizebuilder *sb, upb_bytesink *out); // The upb_sink to which data can be sent to be encoded. Note that this data // must be identical to the data that was previously given to the sizebuilder // (if any). -upb_sink *upb_encoder_sink(upb_encoder *s); +upb_sink *upb_encoder_sink(upb_encoder *e); #ifdef __cplusplus } /* extern "C" */ diff --git a/src/upb_sink.h b/src/upb_sink.h index 41e527553b..2b8369e7d9 100644 --- a/src/upb_sink.h +++ b/src/upb_sink.h @@ -66,7 +66,7 @@ typedef struct { // The value callback is called for a regular value (ie. not a string or // submessage). typedef upb_sink_status (*upb_value_cb)(upb_sink *s, upb_fielddef *f, - upb_value val); + upb_value val, upb_status *status); // The string callback is called for string data. "str" is the string in which // the data lives, but it may contain more data than the effective string. @@ -81,12 +81,15 @@ typedef upb_sink_status (*upb_value_cb)(upb_sink *s, upb_fielddef *f, // copying if it is unavoidable. typedef upb_sink_status (*upb_str_cb)(upb_sink *s, upb_fielddef *f, upb_strptr str, - int32_t start, uint32_t end); + int32_t start, uint32_t end, + upb_status *status); // The start and end callbacks are called when a submessage begins and ends, // respectively. -typedef upb_sink_status (*upb_start_cb)(upb_sink *s, upb_fielddef *f); -typedef upb_sink_status (*upb_end_cb)(upb_sink *s, upb_fielddef *f); +typedef upb_sink_status (*upb_start_cb)(upb_sink *s, upb_fielddef *f, + upb_status *status); +typedef upb_sink_status (*upb_end_cb)(upb_sink *s, upb_fielddef *f, + upb_status *status); /* upb_sink implementation ****************************************************/ @@ -109,10 +112,10 @@ typedef struct upb_sink_callbacks { // possible to write C++ sinks in a more natural style without loss of // efficiency. We could have a flag in upb_sink defining whether it is a C // sink or a C++ one. -#define upb_sink_onvalue(s, f, val) s->vtbl->value_cb(s, f, val) -#define upb_sink_onstr(s, f, str, start, end) s->vtbl->str_cb(s, f, str, start, end) -#define upb_sink_onstart(s, f) s->vtbl->start_cb(s, f) -#define upb_sink_onend(s, f) s->vtbl->end_cb(s, f) +#define upb_sink_onvalue(s, f, val, status) s->vtbl->value_cb(s, f, val, status) +#define upb_sink_onstr(s, f, str, start, end, status) s->vtbl->str_cb(s, f, str, start, end, status) +#define upb_sink_onstart(s, f, status) s->vtbl->start_cb(s, f, status) +#define upb_sink_onend(s, f, status) s->vtbl->end_cb(s, f, status) // Initializes a plain C visitor with the given vtbl. The sink must have been // allocated separately. @@ -134,9 +137,11 @@ INLINE void upb_sink_init(upb_sink *s, upb_sink_callbacks *vtbl) { struct _upb_bytesink; // The single bytesink callback; it takes the bytes to be written and returns -// how many were successfully written. If zero is returned, it indicates that -// no more bytes can be accepted right now. -typedef size_t (*upb_byte_cb)(struct _upb_bytesink *s, upb_strptr str); +// how many were successfully written. If the return value is <0, the caller +// should stop processing. +typedef int32_t (*upb_byte_cb)(struct _upb_bytesink *s, upb_strptr str, + uint32_t start, uint32_t end, + upb_status *status); typedef struct _upb_bytesink { upb_byte_cb *cb;