Re: Silent change to string.pack("cn",...) between 5.3.1 and 5.3.2

[Duane Leslie <parakleta@...>]

On 8 April 2016 at 14:19, Duane Leslie <parakleta@darkreality.org> wrote:
>> On 7 April 2016 at 13:34, Duane Leslie <parakleta@darkreality.org> wrote:
>>> LUA_PACKPADBYTE (not necessarily '\0')
>>>
>>> The padding option is definitely convenient but I dislike padding with LUA_PACKPADBYTE rather than '\0'
>>
>> LUA_PACKPADBYTE *is* defined as '\0'.
>> If you're defining it to something else, don't complain that it's different....
>
> My issue is not so much what it may or may not be defined as, but
> rather that semantically it is not the same as '\0' and this has the
> flow on effect that when it is silently inserted by `pack()` it is
> then loudly returned by `unpack()`.
>
> In all other cases `unpack()` swallows any LUA_PACKPADBYTE but it
> cannot in the case of the fixed string format because it allows any
> data to be inserted and so cannot differentiate padding bytes from
> legitimate data.  If you pad the zero terminated string with '\0' then
> you can remove them when unpacking because the semantics are clear.
>
> Regards,
>
> Duane.

Actually, this consideration of semantics makes me realise that my
patch contains an error, it should instead have:

@@ -1367,9 +1370,12 @@ static int str_pack (lua_State *L) {
         size_t len;
         const char *s = luaL_checklstring(L, arg, &len);
         luaL_argcheck(L, strlen(s) == len, arg, "string contains zeros");
+        luaL_argcheck(L, size == 0 || len < (size_t)size, arg,
"string is too long");
         luaL_addlstring(&b, s, len);
         luaL_addchar(&b, '\0');  /* add zero at the end */
-        totalsize += len + 1;
+        while (++len < (size_t)size)  /* pad extra space */
+          luaL_addchar(&b, LUA_PACKPADBYTE);
+        totalsize += len;
         break;
       }
       case Kpadding: luaL_addchar(&b, LUA_PACKPADBYTE);  /* FALLTHROUGH */