Re: Bug in lstring.c and/or lobject.h?

[Roberto Ierusalimschy <roberto@...>]

> svalue() is defined as (lobject.h):
> 
> #define svalue(o)		getstr(tsvalue(o))
> with
> #define getstr(ts)      cast(const char *, (ts) + 1)
> 
> If I manually resolve this macro a bit, I get (barring calls to
> check_exp)
> 
> #define svalue(o)		(const char *)(
> &((o)->value.gc->ts.tsv)+ 1)
> 
> In other words, it returns (char *)((o)->value.gc->ts.tsv) +
> sizeof(Tstring.stv), with o a TObject *, as the start of the actual
> string.
> 
> However, it is initialized differently, see 
> 
> static TSTring *newlstr(lua_State *L, const char *str, size_t l,
> unsigned int h) in lstring.c:
> 
> TString *ts;
> [..]
> ts = cast(TString *, luaM_malloc(L,
> (l+1)*sizeof(char)+sizeof(TString)));
> ts->tsv.len = l;
> ts->tsv.hash = h;
> ts->tsv.marked = luaC_white(G(L));
> ts->tsv.tt = LUA_TSTRING;
> ts->tsv.reserved = 0;
> memcpy(ts+1, str, l*sizeof(char));
> [..]
> return ts;
> 
> In other words, the string starts at (char *) ts + sizeof(TString).
> However, svalue expects it to be at (char *)ts.tsv +
> sizeof(TString.tsv). This might not be the same!

I guess you are right.

Actually, the current definition of 'svalue' misses the whole point of
using the union to ensure alignment.


> I can "fix" things by adding another unsigned int to TString, but I
> can't imagine that this is the right approach. Another option would be
> to rewrite getstr(), so that it reads something like (cast(const char *,
> (ts)) + sizeof(TString) but neither sounds very appealing. Maybe the
> best option would be to rewrite the memcpy from newlstr to memcpy (
> (char *)ts + sizeof(ts->tsv), ...)

It seems that the best option (I would say the correct one) is to change
the definition of svalue to this:

  #define svalue(o)       getstr(rawtsvalue(o))

Also, we should make sure that all other uses of getstr apply only to
raw svalues.

-- Roberto