From 6b574175dd43735a26f171de6b34c3f15bb50f12 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Tue, 22 Feb 2011 11:31:32 -0800 Subject: Prevent the default message from getting mutated. If a Lua program references the default message, it will be copied into a mutable object. --- benchmarks/google_messages.proto | 2 +- lang_ext/lua/test.lua | 12 +++++++----- lang_ext/lua/upb.c | 2 +- src/upb_msg.c | 20 ++++++++++++++++++++ src/upb_msg.h | 19 ++++++++++++++++++- 5 files changed, 47 insertions(+), 8 deletions(-) diff --git a/benchmarks/google_messages.proto b/benchmarks/google_messages.proto index 17cace7..b43e94b 100644 --- a/benchmarks/google_messages.proto +++ b/benchmarks/google_messages.proto @@ -51,7 +51,7 @@ message SpeedMessage1SubMessage { optional int32 field1 = 1 [default=0]; optional int32 field2 = 2 [default=0]; optional int32 field3 = 3 [default=0]; - optional string field15 = 15; + optional string field15 = 15 [default="FOOBAR!"]; optional bool field12 = 12 [default=true]; optional int64 field13 = 13; optional int64 field14 = 14; diff --git a/lang_ext/lua/test.lua b/lang_ext/lua/test.lua index 978fb11..67c955e 100644 --- a/lang_ext/lua/test.lua +++ b/lang_ext/lua/test.lua @@ -23,15 +23,17 @@ SpeedMessage1 = symtab:lookup("benchmarks.SpeedMessage1") print(SpeedMessage1:name()) msg = SpeedMessage1() --- print(msg.field1) +--print(msg.field1) -- print(msg.field129) -- print(msg.field271) --- print(msg.field15.field15) +print(msg.field15.field15) +msg.field15.field15 = "my override" +print(msg.field15.field15) -- print(msg.field1) -- print(msg.field1) -- msg.field1 = "YEAH BABY!" -- print(msg.field1) -print(msg.field129) -msg.field129 = 5 -print(msg.field129) +-- print(msg.field129) +-- msg.field129 = 5 +-- print(msg.field129) diff --git a/lang_ext/lua/upb.c b/lang_ext/lua/upb.c index 460ac86..2e0102e 100644 --- a/lang_ext/lua/upb.c +++ b/lang_ext/lua/upb.c @@ -170,7 +170,7 @@ static void lupb_pushvalue(lua_State *L, upb_value val, upb_fielddef *f) { static upb_value lupb_getvalue(lua_State *L, int narg, upb_fielddef *f) { upb_value val; - lua_Number num; + lua_Number num = 0; if (!upb_issubmsg(f) && !upb_isstring(f) && f->type != UPB_TYPE(BOOL)) { num = luaL_checknumber(L, narg); if (f->type != UPB_TYPE(DOUBLE) && f->type != UPB_TYPE(FLOAT) && diff --git a/src/upb_msg.c b/src/upb_msg.c index 211004c..27170f5 100644 --- a/src/upb_msg.c +++ b/src/upb_msg.c @@ -161,6 +161,26 @@ void upb_msg_set(upb_msg *msg, upb_fielddef *f, upb_value val) { return upb_value_write(ptr, val, upb_field_valuetype(f)); } +upb_value upb_msg_get(upb_msg *msg, upb_fielddef *f) { + if (!upb_msg_has(msg, f)) { + upb_value val = f->default_value; + if (upb_issubmsg(f)) { + // TODO: handle arrays also, which must be treated similarly. + upb_msgdef *md = upb_downcast_msgdef(f->def); + upb_msg *m = upb_msg_new(md); + // Copy all set bits and values, except the refcount. + memcpy(m , upb_value_getmsg(val), md->size); + upb_atomic_refcount_init(&m->refcount, 0); // The msg will take a ref. + upb_value_setmsg(&val, m); + } + upb_msg_set(msg, f, val); + return val; + } else { + return upb_value_read(_upb_msg_getptr(msg, f), upb_field_valuetype(f)); + } +} + + static upb_valueptr upb_msg_getappendptr(upb_msg *msg, upb_fielddef *f) { upb_valueptr p = _upb_msg_getptr(msg, f); if (upb_isarray(f)) { diff --git a/src/upb_msg.h b/src/upb_msg.h index ff8489c..468af24 100644 --- a/src/upb_msg.h +++ b/src/upb_msg.h @@ -10,6 +10,8 @@ * * upb's parsers and serializers could also be used to populate and serialize * other kinds of message objects (even one generated by Google's protobuf). + * + * TODO: consider properly supporting const instances. */ #ifndef UPB_MSG_H @@ -204,12 +206,16 @@ upb_msg *upb_msg_new(upb_msgdef *md); INLINE void upb_msg_unref(upb_msg *msg, upb_msgdef *md) { if (msg && upb_atomic_unref(&msg->refcount)) _upb_msg_free(msg, md); } + INLINE upb_msg *upb_msg_getref(upb_msg *msg) { assert(msg); upb_atomic_ref(&msg->refcount); return msg; } +// Modifies *msg to point to a newly initialized msg instance. If the msg had +// no other referents, reuses the same msg, otherwise allocates a new one. +// The caller *must* own a ref on the msg prior to calling this method! void upb_msg_recycle(upb_msg **msg, upb_msgdef *msgdef); // Tests whether the given field is explicitly set, or whether it will return a @@ -240,9 +246,17 @@ INLINE bool upb_msg_has(upb_msg *msg, upb_fielddef *f) { // // For the moment we go with (2). Google's protobuf does (3), which is likely // part of the reason we beat it in some benchmarks. +// +// If the branchiness of (2) is too great, this could be mitigated with cmov +// (both values and the conditional are cheap to calculate, much cheaper than +// the cost of a misprediction). // For submessages and strings, the returned value is not owned. -INLINE upb_value upb_msg_get(upb_msg *msg, upb_fielddef *f) { +upb_value upb_msg_get(upb_msg *msg, upb_fielddef *f); + +// A specialized version of the previous that is cheaper because it doesn't +// support submessages or arrays. +INLINE upb_value upb_msg_getscalar(upb_msg *msg, upb_fielddef *f) { if (upb_msg_has(msg, f)) { return upb_value_read(_upb_msg_getptr(msg, f), upb_field_valuetype(f)); } else { @@ -250,6 +264,9 @@ INLINE upb_value upb_msg_get(upb_msg *msg, upb_fielddef *f) { } } +// Sets the given field to the given value. If the field is a string, array, +// or submessage, releases the ref on any object we may have been referencing +// and takes a ref on the new object (if any). void upb_msg_set(upb_msg *msg, upb_fielddef *f, upb_value val); // Unsets all field values back to their defaults. -- cgit v1.2.3