From 87b2c69c15716b96a294f5918878fb8b7b9a0b40 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 17 Jul 2010 12:56:04 -0700 Subject: Fleshed out upb_stdio and upb_textprinter. test_decoder now compiles and links! But it doesn't work yet. --- stream/upb_textprinter.h | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 stream/upb_textprinter.h (limited to 'stream/upb_textprinter.h') diff --git a/stream/upb_textprinter.h b/stream/upb_textprinter.h new file mode 100644 index 0000000..7e35412 --- /dev/null +++ b/stream/upb_textprinter.h @@ -0,0 +1,30 @@ +/* + * upb - a minimalist implementation of protocol buffers. + * + * Copyright (c) 2009 Joshua Haberman. See LICENSE for details. + */ + +#ifndef UPB_TEXT_H_ +#define UPB_TEXT_H_ + +#include "upb_stream.h" + +#ifdef __cplusplus +extern "C" { +#endif + +struct _upb_textprinter; +typedef struct _upb_textprinter upb_textprinter; + +upb_textprinter *upb_textprinter_new(); +void upb_textprinter_free(upb_textprinter *p); +void upb_textprinter_reset(upb_textprinter *p, upb_bytesink *sink, + bool single_line); + +upb_sink *upb_textprinter_sink(upb_textprinter *p); + +#ifdef __cplusplus +} /* extern "C" */ +#endif + +#endif /* UPB_TEXT_H_ */ -- cgit v1.2.3 From 2c24cbb108bbda296f01e7628028b1dcb2b9516b Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Tue, 25 Jan 2011 10:07:47 -0800 Subject: More work on decoder and stdio bytesrc/bytesink. --- Makefile | 6 ++--- core/upb.c | 17 +++++-------- core/upb_stream.h | 16 +++++++----- stream/upb_decoder.c | 14 +++++++--- stream/upb_stdio.c | 66 +++++++++++++++++++++++------------------------- stream/upb_textprinter.c | 1 - stream/upb_textprinter.h | 3 +-- 7 files changed, 63 insertions(+), 60 deletions(-) (limited to 'stream/upb_textprinter.h') diff --git a/Makefile b/Makefile index 46cb836..1dfd79d 100644 --- a/Makefile +++ b/Makefile @@ -63,10 +63,10 @@ SRC=core/upb.c \ descriptor/descriptor.c \ core/upb_def.c \ stream/upb_decoder.c \ + stream/upb_stdio.c \ + stream/upb_textprinter.c # core/upb_msg.c \ -# stream/upb_stdio.c \ # stream/upb_strstream.c \ -# stream/upb_textprinter.c $(SRC): perf-cppflags # Parts of core that are yet to be converted. @@ -114,7 +114,7 @@ TESTS=tests/test_string \ tests/test_table \ tests/test_def \ tests/test_stream \ -# tests/test_decoder \ + tests/test_decoder \ # tests/t.test_vs_proto2.googlemessage1 \ # tests/t.test_vs_proto2.googlemessage2 \ # tests/test.proto.pb diff --git a/core/upb.c b/core/upb.c index ff2d47e..525c8a8 100644 --- a/core/upb.c +++ b/core/upb.c @@ -41,16 +41,13 @@ const upb_type_info upb_types[] = { }; void upb_seterr(upb_status *status, enum upb_status_code code, - const char *msg, ...) -{ - if(upb_ok(status)) { // The first error is the most interesting. - status->code = code; - upb_string_recycle(&status->str); - va_list args; - va_start(args, msg); - upb_string_vprintf(status->str, msg, args); - va_end(args); - } + const char *msg, ...) { + status->code = code; + upb_string_recycle(&status->str); + va_list args; + va_start(args, msg); + upb_string_vprintf(status->str, msg, args); + va_end(args); } void upb_copyerr(upb_status *to, upb_status *from) diff --git a/core/upb_stream.h b/core/upb_stream.h index bf312a8..d0045cc 100644 --- a/core/upb_stream.h +++ b/core/upb_stream.h @@ -178,12 +178,16 @@ INLINE void upb_src_run(upb_src *src, upb_status *status); INLINE upb_strlen_t upb_bytesrc_read(upb_bytesrc *src, void *buf, upb_strlen_t count, upb_status *status); -// Like upb_bytesrc_read(), but modifies "str" in-place, possibly aliasing -// existing string data (which avoids a copy). On the other hand, if -// the data was *not* already in an existing string, this copies it into -// a upb_string, and if the data needs to be put in a specific range of -// memory (because eg. you need to put it into a different kind of string -// object) then upb_bytesrc_get() could be better. +// Like upb_bytesrc_read(), but modifies "str" in-place. "str" MUST be newly +// created or just recycled. Returns "false" if no data was returned, either +// due to error or EOF (check status for details). +// +// In comparison to upb_bytesrc_read(), this call can possibly alias existing +// string data (which avoids a copy). On the other hand, if the data was *not* +// already in an existing string, this copies it into a upb_string, and if the +// data needs to be put in a specific range of memory (because eg. you need to +// put it into a different kind of string object) then upb_bytesrc_get() could +// be better. INLINE bool upb_bytesrc_getstr(upb_bytesrc *src, upb_string *str, upb_status *status); diff --git a/stream/upb_decoder.c b/stream/upb_decoder.c index b4b32ff..e60915f 100644 --- a/stream/upb_decoder.c +++ b/stream/upb_decoder.c @@ -126,6 +126,7 @@ static bool upb_getbuf(upb_decoder *d, void *data, size_t bytes_wanted, } else { // End-of-buffer. if (d->buf) d->buf_stream_offset += upb_string_len(d->buf); + upb_string_recycle(&d->buf); if (!upb_bytesrc_getstr(d->bytesrc, d->buf, d->status)) return false; s->ptr = upb_string_getrobuf(d->buf); } @@ -295,7 +296,15 @@ void upb_decoder_run(upb_src *src, upb_status *status) { while(1) { // Parse/handle tag. upb_tag tag; - CHECK(upb_decode_tag(d, &state, &tag)); + if (!upb_decode_tag(d, &state, &tag)) { + if (status->code == UPB_EOF && d->top == d->stack) { + // Normal end-of-file. + CHECK_FLOW(upb_dispatch_endmsg(&d->dispatcher)); + return; + } else { + goto err; + } + } // Decode wire data. Hopefully this branch will predict pretty well // since most types will read a varint here. @@ -361,9 +370,6 @@ void upb_decoder_run(upb_src *src, upb_status *status) { CHECK_FLOW(upb_dispatch_value(&d->dispatcher, f, val)); } - CHECK_FLOW(upb_dispatch_endmsg(&d->dispatcher)); - return; - err: if (upb_ok(status)) { upb_seterr(status, UPB_ERROR, "Callback returned UPB_BREAK"); diff --git a/stream/upb_stdio.c b/stream/upb_stdio.c index 820399b..7923664 100644 --- a/stream/upb_stdio.c +++ b/stream/upb_stdio.c @@ -23,44 +23,42 @@ void upb_stdio_reset(upb_stdio *stdio, FILE* file) { stdio->file = file; } -static bool upb_stdio_read(upb_stdio *stdio, upb_string *str, - int offset, size_t bytes_to_read) { - char *buf = upb_string_getrwbuf(str, offset + bytes_to_read) + offset; - size_t read = fread(buf, 1, bytes_to_read, stdio->file); - if(read < bytes_to_read) { +static upb_strlen_t upb_stdio_read(upb_bytesrc *src, void *buf, + upb_strlen_t count, upb_status *status) { + upb_stdio *stdio = (upb_stdio*)src; + assert(count > 0); + size_t read = fread(buf, 1, count, stdio->file); + if(read < (size_t)count) { // Error or EOF. - stdio->bytesrc.eof = feof(stdio->file); - if(ferror(stdio->file)) { - upb_seterr(&stdio->bytesrc.status, UPB_STATUS_ERROR, - "Error reading from stdio stream."); - return false; + if(feof(stdio->file)) { + upb_seterr(status, UPB_EOF, ""); + return read; + } else if(ferror(stdio->file)) { + upb_seterr(status, UPB_ERROR, "Error reading from stdio stream."); + return -1; } - // Resize to actual read size. - upb_string_getrwbuf(str, offset + read); } - return true; + return read; } -bool upb_stdio_get(upb_bytesrc *src, upb_string *str, upb_strlen_t minlen) { - // We ignore "minlen" since the stdio interfaces always return a full read - // unless they are at EOF. - (void)minlen; - return upb_stdio_read((upb_stdio*)src, str, 0, BLOCK_SIZE); -} - -bool upb_stdio_append(upb_bytesrc *src, upb_string *str, upb_strlen_t len) { - return upb_stdio_read((upb_stdio*)src, str, upb_string_len(str), len); +static bool upb_stdio_getstr(upb_bytesrc *src, upb_string *str, + upb_status *status) { + upb_strlen_t read = upb_stdio_read( + src, upb_string_getrwbuf(str, BLOCK_SIZE), BLOCK_SIZE, status); + if (read <= 0) return false; + upb_string_getrwbuf(str, read); + return true; } int32_t upb_stdio_put(upb_bytesink *sink, upb_string *str) { upb_stdio *stdio = (upb_stdio*)((char*)sink - offsetof(upb_stdio, bytesink)); upb_strlen_t len = upb_string_len(str); - size_t written = fwrite(upb_string_getrobuf(str), 1, len, stdio->file); + upb_strlen_t written = fwrite(upb_string_getrobuf(str), 1, len, stdio->file); if(written < len) { // Error or EOF. stdio->bytesink.eof = feof(stdio->file); if(ferror(stdio->file)) { - upb_seterr(&stdio->bytesink.status, UPB_STATUS_ERROR, + upb_seterr(&stdio->bytesink.status, UPB_ERROR, "Error writing to stdio stream."); return 0; } @@ -68,19 +66,19 @@ int32_t upb_stdio_put(upb_bytesink *sink, upb_string *str) { return written; } -static upb_bytesrc_vtable upb_stdio_bytesrc_vtbl = { - (upb_bytesrc_get_fptr)upb_stdio_get, - (upb_bytesrc_append_fptr)upb_stdio_append, -}; +upb_stdio *upb_stdio_new() { + static upb_bytesrc_vtbl bytesrc_vtbl = { + upb_stdio_read, + upb_stdio_getstr, + }; -static upb_bytesink_vtable upb_stdio_bytesink_vtbl = { - upb_stdio_put -}; + //static upb_bytesink_vtbl bytesink_vtbl = { + // upb_stdio_put + //}; -upb_stdio *upb_stdio_new() { upb_stdio *stdio = malloc(sizeof(*stdio)); - upb_bytesrc_init(&stdio->bytesrc, &upb_stdio_bytesrc_vtbl); - upb_bytesink_init(&stdio->bytesink, &upb_stdio_bytesink_vtbl); + upb_bytesrc_init(&stdio->bytesrc, &bytesrc_vtbl); + //upb_bytesink_init(&stdio->bytesink, &bytesink_vtbl); return stdio; } diff --git a/stream/upb_textprinter.c b/stream/upb_textprinter.c index 2d2e237..3a77ab1 100644 --- a/stream/upb_textprinter.c +++ b/stream/upb_textprinter.c @@ -12,7 +12,6 @@ #include "upb_string.h" struct _upb_textprinter { - upb_sink sink; upb_bytesink *bytesink; upb_string *str; int indent_depth; diff --git a/stream/upb_textprinter.h b/stream/upb_textprinter.h index 7e35412..b40d9fa 100644 --- a/stream/upb_textprinter.h +++ b/stream/upb_textprinter.h @@ -20,8 +20,7 @@ upb_textprinter *upb_textprinter_new(); void upb_textprinter_free(upb_textprinter *p); void upb_textprinter_reset(upb_textprinter *p, upb_bytesink *sink, bool single_line); - -upb_sink *upb_textprinter_sink(upb_textprinter *p); +void upb_textprinter_sethandlers(upb_textprinter *p, upb_handlers *h); #ifdef __cplusplus } /* extern "C" */ -- cgit v1.2.3 From d98db7cb567f17a3bb56e2af8499d2e3aef03b3b Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 29 Jan 2011 12:07:09 -0800 Subject: Textprinter is compiling again. --- core/upb_stream.h | 22 ++++++--- core/upb_stream_vtbl.h | 21 ++++++--- stream/upb_textprinter.c | 114 ++++++++++++++++++++++++++++------------------- stream/upb_textprinter.h | 4 +- 4 files changed, 103 insertions(+), 58 deletions(-) (limited to 'stream/upb_textprinter.h') diff --git a/core/upb_stream.h b/core/upb_stream.h index 09e4025..aa23549 100644 --- a/core/upb_stream.h +++ b/core/upb_stream.h @@ -245,16 +245,26 @@ INLINE bool upb_value_getfullstr(upb_value val, upb_string *str, struct _upb_bytesink; typedef struct _upb_bytesink upb_bytesink; -INLINE bool upb_bytesink_printf(upb_bytesink *sink, const char *fmt, ...); +// TODO: Figure out how buffering should be handled. Should the caller buffer +// data and only call these functions when a buffer is full? Seems most +// efficient, but then buffering has to be configured in the caller, which +// could be anything, which makes it hard to have a standard interface for +// controlling buffering. +// +// The downside of having the bytesink buffer is efficiency: the caller is +// making more (virtual) function calls, and the caller can't arrange to have +// a big contiguous buffer. The bytesink can do this, but will have to copy +// to make the data contiguous. + +// Returns the number of bytes written. +INLINE upb_strlen_t upb_bytesink_printf(upb_bytesink *sink, upb_status *status, + const char *fmt, ...); // Puts the given string, returning true if the operation was successful, otherwise // check "status" for details. Ownership of the string is *not* passed; if // the callee wants a reference he must call upb_string_getref() on it. -INLINE bool upb_bytesink_putstr(upb_bytesink *sink, upb_string *str, - upb_status *status); - -// Returns the current error status for the stream. -INLINE upb_status *upb_bytesink_status(upb_bytesink *sink); +INLINE upb_strlen_t upb_bytesink_putstr(upb_bytesink *sink, upb_string *str, + upb_status *status); #include "upb_stream_vtbl.h" diff --git a/core/upb_stream_vtbl.h b/core/upb_stream_vtbl.h index ef655fd..8e8971f 100644 --- a/core/upb_stream_vtbl.h +++ b/core/upb_stream_vtbl.h @@ -34,8 +34,10 @@ typedef bool (*upb_bytesrc_getstr_fptr)( // upb_bytesink. typedef upb_strlen_t (*upb_bytesink_write_fptr)( upb_bytesink *bytesink, void *buf, upb_strlen_t count); -typedef upb_strlen_t (*upb_bytesink_putstr_fptr)( - upb_bytesink *bytesink, upb_string *str); +typedef bool (*upb_bytesink_putstr_fptr)( + upb_bytesink *bytesink, upb_string *str, upb_status *status); +typedef upb_strlen_t (*upb_bytesink_vprintf_fptr)( + upb_status *status, const char *fmt, va_list args); // Vtables for the above interfaces. typedef struct { @@ -44,8 +46,9 @@ typedef struct { } upb_bytesrc_vtbl; typedef struct { - upb_bytesink_write_fptr write; - upb_bytesink_putstr_fptr putstr; + upb_bytesink_write_fptr write; + upb_bytesink_putstr_fptr putstr; + upb_bytesink_vprintf_fptr vprintf; } upb_bytesink_vtbl; typedef struct { @@ -140,13 +143,21 @@ INLINE upb_strlen_t upb_bytesink_write(upb_bytesink *sink, void *buf, } INLINE upb_strlen_t upb_bytesink_putstr(upb_bytesink *sink, upb_string *str, upb_status *status) { - return sink->vtbl->putstr(sink, str); + return sink->vtbl->putstr(sink, str, status); } INLINE upb_status *upb_bytesink_status(upb_bytesink *sink) { return &sink->status; } +INLINE upb_strlen_t upb_bytesink_printf(upb_bytesink *sink, upb_status *status, const char *fmt, ...) { + va_list args; + va_start(args, fmt); + upb_strlen_t ret = sink->vtbl->vprintf(status, fmt, args); + va_end(args); + return ret; +} + // upb_handlers struct _upb_handlers { upb_handlerset *set; diff --git a/stream/upb_textprinter.c b/stream/upb_textprinter.c index 2209173..7025494 100644 --- a/stream/upb_textprinter.c +++ b/stream/upb_textprinter.c @@ -15,34 +15,51 @@ struct _upb_textprinter { upb_bytesink *bytesink; int indent_depth; bool single_line; - upb_fielddef *f; + upb_status status; }; -static void upb_textprinter_indent(upb_textprinter *p) +#define CHECK(x) if ((x) < 0) goto err; + +static int upb_textprinter_indent(upb_textprinter *p) { if(!p->single_line) for(int i = 0; i < p->indent_depth; i++) - upb_bytesink_put(p->bytesink, UPB_STRLIT(" ")); + CHECK(upb_bytesink_putstr(p->bytesink, UPB_STRLIT(" "), &p->status)); + return 0; +err: + return -1; } -static void upb_textprinter_endfield(upb_textprinter *p) { - if(p->single_line) - upb_bytesink_put(p->bytesink, UPB_STRLIT(" ")); - else - upb_bytesink_put(p->bytesink, UPB_STRLIT("\n")); +static int upb_textprinter_startfield(upb_textprinter *p, upb_fielddef *f) { + upb_textprinter_indent(p); + CHECK(upb_bytesink_printf(p->bytesink, &p->status, UPB_STRFMT ": ", UPB_STRARG(f->name))); + return 0; +err: + return -1; +} + +static int upb_textprinter_endfield(upb_textprinter *p) { + if(p->single_line) { + CHECK(upb_bytesink_putstr(p->bytesink, UPB_STRLIT(" "), &p->status)); + } else { + CHECK(upb_bytesink_putstr(p->bytesink, UPB_STRLIT("\n"), &p->status)); + } + return 0; +err: + return -1; } static upb_flow_t upb_textprinter_value(void *_p, upb_fielddef *f, upb_value val) { upb_textprinter *p = _p; - upb_textprinter_indent(p); - upb_bytesink_printf(p->bytesink, UPB_STRFMT ": ", UPB_STRARG(f->name)); -#define CASE(fmtstr, member) upb_bytesink_printf(p->bytesink, fmtstr, val.member); break; - switch(p->f->type) { + upb_textprinter_startfield(p, f); +#define CASE(fmtstr, member) \ + CHECK(upb_bytesink_printf(p->bytesink, &p->status, fmtstr, upb_value_get ## member(val))); break; + switch(f->type) { case UPB_TYPE(DOUBLE): - CASE("%0.f", _double); + CASE("%0.f", double); case UPB_TYPE(FLOAT): - CASE("%0.f", _float) + CASE("%0.f", float) case UPB_TYPE(INT64): case UPB_TYPE(SFIXED64): case UPB_TYPE(SINT64): @@ -50,40 +67,48 @@ static upb_flow_t upb_textprinter_value(void *_p, upb_fielddef *f, case UPB_TYPE(UINT64): case UPB_TYPE(FIXED64): CASE("%" PRIu64, uint64) - case UPB_TYPE(INT32): - case UPB_TYPE(SFIXED32): - case UPB_TYPE(SINT32): - CASE("%" PRId32, int32) case UPB_TYPE(UINT32): case UPB_TYPE(FIXED32): CASE("%" PRIu32, uint32); case UPB_TYPE(ENUM): { - upb_enumdef *enum_def; - upb_string *enum_label; - (enum_def = upb_downcast_enumdef(p->f->def)) != NULL && - (enum_label = upb_enumdef_iton(enum_def, val.int32)) != NULL) { - // This is an enum value for which we found a corresponding string. - upb_bytesink_put(p->bytesink, enum_label); - CASE("%" PRIu32, uint32); + upb_enumdef *enum_def = upb_downcast_enumdef(f->def); + upb_string *enum_label = + upb_enumdef_iton(enum_def, upb_value_getint32(val)); + if (enum_label) { + // We found a corresponding string for this enum. Otherwise we fall + // through to the int32 code path. + CHECK(upb_bytesink_putstr(p->bytesink, enum_label, &p->status)); + break; + } } + case UPB_TYPE(INT32): + case UPB_TYPE(SFIXED32): + case UPB_TYPE(SINT32): + CASE("%" PRId32, int32) case UPB_TYPE(BOOL): - CASE("%hhu", _bool); + CASE("%hhu", bool); case UPB_TYPE(STRING): case UPB_TYPE(BYTES): - upb_bytesink_put(p->bytesink, UPB_STRLIT(": \"")); - upb_bytesink_put(p->bytesink, str); - upb_bytesink_put(p->bytesink, UPB_STRLIT("\"")); + // TODO: escaping. + CHECK(upb_bytesink_putstr(p->bytesink, UPB_STRLIT(": \""), &p->status)); + CHECK(upb_bytesink_putstr(p->bytesink, upb_value_getstr(val), &p->status)) + CHECK(upb_bytesink_putstr(p->bytesink, UPB_STRLIT("\""), &p->status)); break; } upb_textprinter_endfield(p); return UPB_CONTINUE; +err: + return UPB_BREAK; } -static upb_flow_t upb_textprinter_startsubmsg(void *_p, upb_fielddef *f) { +static upb_flow_t upb_textprinter_startsubmsg(void *_p, upb_fielddef *f, + upb_handlers *delegate_to) { + (void)delegate_to; upb_textprinter *p = _p; + upb_textprinter_startfield(p, f); p->indent_depth++; - upb_bytesink_put(p->bytesink, UPB_STRLIT(" {")); - if(!p->single_line) upb_bytesink_put(p->bytesink, UPB_STRLIT("\n")); + upb_bytesink_putstr(p->bytesink, UPB_STRLIT(" {"), &p->status); + if(!p->single_line) upb_bytesink_putstr(p->bytesink, UPB_STRLIT("\n"), &p->status); return UPB_CONTINUE; } @@ -92,21 +117,13 @@ static upb_flow_t upb_textprinter_endsubmsg(void *_p) upb_textprinter *p = _p; p->indent_depth--; upb_textprinter_indent(p); - upb_bytesink_put(p->bytesink, UPB_STRLIT("}")); + upb_bytesink_putstr(p->bytesink, UPB_STRLIT("}"), &p->status); upb_textprinter_endfield(p); return UPB_CONTINUE; } upb_textprinter *upb_textprinter_new() { - static upb_handlerset handlers = { - NULL, // startmsg - NULL, // endmsg - upb_textprinter_putval, - upb_textprinter_startsubmsg, - upb_textprinter_endsubmsg, - }; upb_textprinter *p = malloc(sizeof(*p)); - upb_byte_init(&p->sink, &upb_textprinter_vtbl); return p; } @@ -114,11 +131,18 @@ void upb_textprinter_free(upb_textprinter *p) { free(p); } -void upb_textprinter_reset(upb_textprinter *p, upb_bytesink *sink, - bool single_line) { +void upb_textprinter_reset(upb_textprinter *p, upb_handlers *handlers, + upb_bytesink *sink, bool single_line) { + static upb_handlerset handlerset = { + NULL, // startmsg + NULL, // endmsg + upb_textprinter_value, + upb_textprinter_startsubmsg, + upb_textprinter_endsubmsg, + }; p->bytesink = sink; p->single_line = single_line; p->indent_depth = 0; + upb_register_handlerset(handlers, &handlerset); + upb_set_handler_closure(handlers, p, &p->status); } - -upb_sink *upb_textprinter_sink(upb_textprinter *p) { return &p->sink; } diff --git a/stream/upb_textprinter.h b/stream/upb_textprinter.h index b40d9fa..a880626 100644 --- a/stream/upb_textprinter.h +++ b/stream/upb_textprinter.h @@ -18,8 +18,8 @@ typedef struct _upb_textprinter upb_textprinter; upb_textprinter *upb_textprinter_new(); void upb_textprinter_free(upb_textprinter *p); -void upb_textprinter_reset(upb_textprinter *p, upb_bytesink *sink, - bool single_line); +void upb_textprinter_reset(upb_textprinter *p, upb_handlers *handlers, + upb_bytesink *sink, bool single_line); void upb_textprinter_sethandlers(upb_textprinter *p, upb_handlers *h); #ifdef __cplusplus -- cgit v1.2.3